Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need force reload to make it work on Safari #16

Closed
akring opened this issue Jun 15, 2016 · 24 comments
Closed

Need force reload to make it work on Safari #16

akring opened this issue Jun 15, 2016 · 24 comments

Comments

@akring
Copy link

akring commented Jun 15, 2016

This plugin is awesome on Chrome for every site I need.However this doesn't work for safari well.

Such as Material search result for Google,I can't apply the style unless I use CMD+R to reload the search page.This is very wired because it worked well on Chrome...

Safari: 9.1.1 (11601.6.17)
Mac OS X 10.11.5 (15F34)

@ctrlrsf
Copy link

ctrlrsf commented Jan 6, 2017

I use GitHub Dark and Hacker News Dark and have a similar issue. Sometimes when I go to either of these sites, the Stylish theme doesn't get applied and I see the stock theme. I have to reload (CMD+R) for it to take effect. I haven't found what causes it, but happens 1 or 2 times a day.

@ghost
Copy link

ghost commented Jan 6, 2017

My configuration:

There is a problem where the style isn't being applied. And I can 100% reproduce it:

  • Open a new tab, paste https://www.youtube.com/results?search_query=christopherodd+walking+dead into the address field, and press Enter. The page will be unstyled.
  • Press Cmd+R to reload the page. The page will be unstyled.
  • Press Cmd+R as many times as you want. The page will remain unstyled.
  • Now press Cmd+L and Enter (to select the location bar and press enter to go there). The page will become styled.
  • Press Cmd+R to reload the page. The page will be unstyled again.
  • Press Cmd+L and Enter. The page will become styled.
  • Press Cmd+L and Enter. The page will become styled.
  • Press Cmd+L and Enter. The page will become styled.
  • Press Cmd+L and Enter. The page will become styled.

Etc.

100% reproduced every time.

So whatever is happening with the initial page load and "Cmd+R", it is not applying Stylish styles during that "page reload" event. But it is applying during the manual "reload by pressing enter on URL bar".

@350d

@350d
Copy link
Owner

350d commented Jan 6, 2017

@SteveJobzniak @ctrlrsf Thank you guys for reporting this issue, i'll check it soon.

@ghost
Copy link

ghost commented Jan 6, 2017

@350d Thank you. Don't worry about "soon". It is a new year. Relax and take care. :)

@350d
Copy link
Owner

350d commented Jan 29, 2017

Latest version 1.9.3 should cover this issue

@350d 350d closed this as completed Jan 29, 2017
@akring
Copy link
Author

akring commented Jan 29, 2017

Unfortunately 1.9.3 didn't solve this problem, you can try it with "Baidu Lite".

Safari: 10.0.3 (12602.4.8)
macOS : 10.12.3 (16D32)

@350d
Copy link
Owner

350d commented Jan 29, 2017

ok, try 1.9.4, looks like this website updating DOM all the time. Another issue with this style - its using url prefixes and not the domain names, tabs not covered by this style, right? (these tabs: 新闻贴吧知道音乐图片视频地图文库更多» )

@ghost
Copy link

ghost commented Jan 29, 2017

@350d Thank you so much! 💃 I downloaded 1.9.3 binary first (before 1.9.4).

And followed my steps to attempt to reproduce the bug above in 1.9.3.

Results:

  • Initial page load: Almost 100% success rate. Almost every initial load was styled. The ones that weren't styled seemed to be a rare race condition (maybe Safari bug). The problem was very rare. That's good enough.
  • Cmd-R page reloads: 100% success rate. Those always style the page now!

Time to try 1.9.4 now.

@ghost
Copy link

ghost commented Jan 29, 2017

@350d I read your code for 1.9.3 and 1.9.4, and what you are doing in 1.9.4 seems great: Anytime the page DOM inserts or deletes nodes via JavaScript it checks if all styles are injected.

I just hope that the "checkStyles()" call is ultra-fast. Because modern pages update the DOM a looooot and would be slowed down if checkStyles() is slow.

Anyway, here are my results:

  • Same as 1.9.3. The issue is fixed! ;-)

And I discovered what sometimes screws up the initial page styling even in 1.9.3+. It's some Safari bug I think... These are the steps that happen when the "unstyled initial load" happens: Open a new tab with Cmd-T, press Cmd-V to paste the URL in the URL bar, I see the URL bar briefly flicker and empty itself again and I hear a "bonk" (error sound) and the URL bar is empty, so I press Cmd-V again to paste it and now it works and the URL is filled in, and then I press Enter to go to the URL (the test URL I gave above), and then it will be unstyled.

Every time the initial page load was unstyled was related to the "bonk" error sound in the URL bar of Safari, so I think it's a Safari bug. And that URL bar bug is very rare so it is no problem!

Thanks for fixing the Stylish problems, it is sweet to finally see a dark YouTube skin all the time! :-) 🍔

@350d
Copy link
Owner

350d commented Jan 29, 2017

ok, i've tested new method with DOMSubtreeModified and some rules to fix performance issues. Check 1.9.5

@ghost
Copy link

ghost commented Jan 30, 2017

@350d

Nice. I see that you never react multiple times when "busy" is true (which happens when Stylish is injecting style elements). So that's good!

I think we should also consider doing this, because this would be the biggest performance boost of all:

var domModifiedTimer = null;
d.addEventListener("DOMSubtreeModified", function(event) {
    if (domModifiedTimer) {
        clearTimeout(domModifiedTimer);
    }
    domModifiedTimer = setTimeout([function here], 250);
});

// for [function here], use a reference to a named/static function
// (which takes zero parameters), instead of an anonymous callback,
// to avoid creating a new anonymous function 1000s of times per webpage.

What this does:

  • Imagine that the user is on Facebook.com and they click on a link, which runs some Ajax query and then uses Javascript to create 200 DOM elements. That's 200 calls to DOMSubtreeModified!
  • The timer has a 250 millisecond (0.25 second) timeout so if those 200 elements are inserted super fast, the timer will clear itself 199 times and then the 200th time it will actually run the Stylish code.
  • That's a massive CPU saving, compared to running the whole "checkStyles" code 200 times!

@350d
Copy link
Owner

350d commented Jan 30, 2017

@SteveJobzniak i'm trying to avoid setTimeouts in this case, better way is just align to requestAnimationFrame i guess.
in your example only script can do - just 100 checks for getElementByID * number of styles available for this page, these operations are super fast.

@ghost
Copy link

ghost commented Jan 30, 2017

@350d Okay, I never heard of requestAnimationFrame, but the quick research I did now seems like it triggers when the window repaints. And DOM insertions (on well-coded sites) do not trigger repaints until all nodes are inserted. So I guess that's fine if it works! :)

Why avoiding setTimeout? To avoid the small 0.25s pause in reaction time to page changes? I guess so...

@350d
Copy link
Owner

350d commented Jan 30, 2017

@SteveJobzniak timeouts can stack inside inactive tab for example and then fire all at once when tab is active again.

@ghost
Copy link

ghost commented Jan 30, 2017

@350d Really? Even with "clearTimeout"? The clearTimeout call happens before setTimeout, so there is only 1 active timer at a time, and the timer only gets started when DOMSubtreeModified happens. The nice thing with setTimeout is that it works even on sites that are badly coded (sites that don't code carefully to make sure that all DOM modifications happen in a single repaint).

By the way, here's an idea with requestAnimationFrame:

  • If page has Stylish styles: Add requestAnimationFrame and DOMSubtreeModified callbacks. Otherwise do nothing.
  • When DOMSubtreeModified happens: Set flag "domModified = true"
  • When animation frame happens: if (domModified) { checkStyles...; domModified = false; }

Since Javascript is single-threaded, this will work. And this method is necessary because requestAnimationFrame callback happens 60 times per second (ouch) so it needs a fast way to check itself.

Edit: I just read more (https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame) and it seems requestAnimationFrame happens a single time. So it's fine to use it directly within DOMSubtreeModified with no other variables, I guess!

@akring
Copy link
Author

akring commented Feb 1, 2017

Thank you man! Version 1.9.5 works fine for me now.

@handcoding
Copy link

Thank you man! Version 1.9.5 works fine for me now.

I went to download Stylish 1.9.5 from the main website, but when I went to install stylish.safariextz, I got an error from Safari saying, “An error occurred while installing the extension ‘Stylish’."

an error occurred while installing the extension stylish

Any ideas there?

(For what it’s worth, I’m on El Cap / 10.11.6 with Safari 10.0.3, and I currently have Stylish 1.9.1 installed.)

@350d
Copy link
Owner

350d commented Feb 1, 2017

@handcoding sorry, can't test on El Capitan, but try to uninstall and then install the extension

@handcoding
Copy link

@350d Ah—that seems to have worked!

Just to be on the safe side, I exported my styles before uninstalling the old version of Stylish—and I’m glad that I did since the process of uninstalling 1.9.1 and installing 1.9.5 seems to have cleared out Stylish’s settings.

Oddly enough, I tried to reimport my styles, but Stylish’s Manage screen is now only showing what looks to be blank placeholder styles in each slot that used to show one of my userstyles (?).

stylish - manage

If it may help, here’s the JSON file with my styles that I exported just before uninstalling 1.9.1:
https://www.dropbox.com/s/wk29a2dx4ckl6c0/stylish%20backup%20-%202017-02-01.json?dl=0

@350d
Copy link
Owner

350d commented Feb 1, 2017

@handcoding Try this one
stylish-backup.txt

@handcoding
Copy link

@handcoding Try this one

Oh, you’re a lifesaver, @350d—that worked perfectly!

Can I ask what you did there in case something like this were to come up again? (Or is the problem solely because of something from 1.9.1?)

@350d
Copy link
Owner

350d commented Feb 2, 2017

@handcoding i should think about compatibility issues while importing from old versions. i've changed escaping for json object and upgraded database in new version (update is safe for autoupdated users, only import affected)

@350d
Copy link
Owner

350d commented Feb 2, 2017

@handcoding compatibility fix applied in v 1.9.6

@handcoding
Copy link

compatibility fix applied in v 1.9.6

Right on. 👌🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants