Remove entire html@class if there wasn't any.

[myfonj]

Prior applyTransitionPatch.
hopefully fixes #706, haven't tested yet.

[eight04]

1. This won't work if the page script runs before the timeout event.
   • If the page script adds a class name, it will also be removed.

Maybe we can just drop the class name? Do you remember why we have to add a class to HTML? @tophf @narcolepticinsomniac @Mottie

[tophf]

I don't remember. Right now I'd say there's no need to add any class or id to documentElement at all. To raise the specificity you can use other methods like maybe :root:not(#foo${CSS.escape(document.documentElement.id || 'bar')}) or just add an attribute based on performance.now() and Math.random()

[narcolepticinsomniac]

We do it to prevent transitions from erroneously applying on page load, which is a bug that both FF and Chromium still have. Chromium has gotten a lot better, but I still see it consistently in the popup, and it was always more random on normal pages, so it probably still happens on them as well. Just checked now, and FF still has it pretty bad.

[tophf]

Yeah, the underlying reason is that, of course. BTW see #177 (comment)

[myfonj]

@eight04 you are right, attribute should be removed only if there wasn't any prior init and remained blank after, I've added the check, thanks for headsup.

[narcolepticinsomniac]

Does it mean that currently, the transition patch doesn't work at all for Chrome v72+?

No. These were occasional random failures, and I've only really noticed them in the popup, so it might be specific to it.

Do you have the link to this issue?

Just an observation made while testing and being stuck on v72 forever. Not even sure if it was just 72, or 72+, but the patch was randomly removed early and transitions applied occasionally when I opened the popup. Anecdotally, I'd say it happened more often with many tabs loaded. v72 was a buggy POS, so maybe it was just that. I switched to readyState, which works well in FF, and v72+, but not at all for internal Chromium pages pre-v72. I don't see the harm in a double-check though.

[narcolepticinsomniac]

Yeah, I'm still seeing this in v74 every once in a while. Definitely a heisenbug, because I could try to force it to happen hundreds of times in a row, and not see it. Once it does happen though, if you repeat the action (opening the popup), the frequency goes way up - enough for me to catch a screen record anyway. Seems like a browser glitch, probably caused by the fancy little animation they put on the popup.

This is with the .then(() => { setTimeout(() => { ... version you posted above, keep an eye on the buttons. Changing the function and reloading isn't really a fair demonstration, because simply reloading the extension would likely reset the glitch. The point is, when troubleshooting this previously, I found that readyState === 'complete' resolves the issue, and once I added it, I didn't run into it anymore.

As I mentioned, IIRC, I'm fairly certain that readyState didn't work at all in the popup pre-v72, which was when I started noticing the glitch, so something obviously changed around then. Combining them seems the most reliable option.

[eight04]

Try this:

diff --git a/content/apply.js b/content/apply.js
index cd30cd1c..c3dbaec3 100644
--- a/content/apply.js
+++ b/content/apply.js
@@ -346,26 +346,29 @@ const APPLY = (() => {
   function applyTransitionPatch() {
     // CSS transition bug workaround: since we insert styles asynchronously,
     // the browsers, especially Firefox, may apply all transitions on page load
-    const className = chrome.runtime.id + '-transition-bug-fix';
-    const docId = document.documentElement.id ? '#' + document.documentElement.id : '';
-    document.documentElement.classList.add(className);
     const el = styleInjector.createStyle('transition-patch');
     // FIXME: this will trigger docRootObserver and cause a resort. We should
     // move this function into style-injector.
     document.documentElement.appendChild(el);
     setStyleContent(el, `
-      ${docId}.${CSS.escape(className)}:root * {
+      :root:not(#\\0):not(#\\0) * {
         transition: none !important;
       }
     `)
+      .then(afterPaint)
       .then(() => {
-        setTimeout(() => {
-          el.remove();
-          document.documentElement.classList.remove(className);
-        });
+        el.remove();
       });
   }
 
+  function afterPaint() {
+    return new Promise(resolve => {
+      requestAnimationFrame(() => {
+        setTimeout(resolve);
+      });
+    });
+  }
+
   function orphanCheck(e) {
     if (e && e.detail.method !== 'orphan') {
       return;

[narcolepticinsomniac]

Looks good initially. Code certainly looks like it should be glitch-proof, but I'll test it for a while and try to break it anyway. =)

[narcolepticinsomniac]

The last version LGTM. We should hook it up and merge it.