Skip to content

Fixed readyStateCompleteListeners never flushing when initialized after DOM ready - #15571

Open
KamilDev wants to merge 1 commit into
darkreader:mainfrom
KamilDev:fix/flush-ready-state-complete-listeners
Open

Fixed readyStateCompleteListeners never flushing when initialized after DOM ready#15571
KamilDev wants to merge 1 commit into
darkreader:mainfrom
KamilDev:fix/flush-ready-state-complete-listeners

Conversation

@KamilDev

Copy link
Copy Markdown

Fixes #15570

The readystatechange listener in dom.ts was only registered when the module initialized before readyState "interactive". When the API is enabled from a deferred page script, initialization happens at "interactive", so readyStateCompleteListeners (deferred SVG fill overrides, image loads) never ran. Register the listener while readyState is not yet "complete" instead. The handler already deals with both flush stages, and nothing changes for the extension, which initializes at document_start.

@alexanderby

Copy link
Copy Markdown
Member

Unfortunately this will have negative effect on websites that load slowly (or when the internet connection is slow), as it postpones certain mechanisms until the state is complete (all style sheets and images finished loading). Particularly, this check is used to remove the fallback style, which is inserted to prevent white flashes.

Is your SVG dynamically generated? Dark Reader is supposed to watch for DOM and inline style changes, but maybe there's a bug. Also this SVG analysis is not perfect, maybe switching it off completely will work better for you. E.g. by supplying a "fix" argument like {ignoreImageAnalysis: ['svg *'], invert: ['svg']}. We also still have experimental theme property {immediateModify: true} that disables the ready state chack.

@KamilDev

Copy link
Copy Markdown
Author

Nothing gets postponed, because the change removes no case from the guard, it only adds one. The line decides whether the listener gets registered, nothing else:

if (!isReadyStateComplete()) {          // was !isDOMReady()
    const onReadyStateChange = () => { /* unchanged */ };
    document.addEventListener('readystatechange', onReadyStateChange);
}

and it's negated, so the narrower predicate registers in more cases, not fewer:

export let isDOMReady: () => boolean = () => {
    return document.readyState === 'complete' || document.readyState === 'interactive';
};

export function isReadyStateComplete(): boolean {
    return document.readyState === 'complete';
}

!isDOMReady() is true only at loading. !isReadyStateComplete() is true at loading and at interactive. Every case that registered before still registers, plus one that never did. So no mechanism can fire later than it used to, the fallback style included.

That added case is init at interactive, which is what a deferred script gets. There the listener was never registered, so readyStateCompleteListeners never flushed at all. The SVG fill overrides weren't running late, they weren't running.

The SVG is static markup. overrideInlineStyle queues the fill handling itself because the heuristic needs getBoundingClientRect(), so the watchers can't cover it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SVG fill attributes are not overridden when enable() runs from a deferred script

2 participants