fix: use documentReady instead of documentComplete in HUD methods#4911
Open
xiaolongnk wants to merge 2 commits into
Open
fix: use documentReady instead of documentComplete in HUD methods#4911xiaolongnk wants to merge 2 commits into
xiaolongnk wants to merge 2 commits into
Conversation
…ations copyToClipboard and pasteFromClipboard were awaiting documentComplete() (the page load event) before constructing the HUD iframe. This caused yy / copy commands to silently defer on still-loading pages until all resources finished loading. documentReady() (DOMContentLoaded) is sufficient to inject the HUD iframe into the DOM and is what the init() path itself already uses. This makes copy/paste work immediately when the DOM is available, regardless of whether the page has fully loaded.
show(), showFindMode(), copyToClipboard(), and pasteFromClipboard() all waited for the load event (documentComplete) before initializing the HUD iframe. On a page whose load event is delayed (slow external resources, never-completing requests), pressing yy would silently do nothing: neither the URL was copied nor the 'Yanked' feedback appeared. documentReady() (DOMContentLoaded) is sufficient — the HUD is a chrome-extension iframe overlay independent of the page's resource load. showClipboardUnavailableMessage is left on documentComplete since it is only shown after a copy attempt already completed.
xiaolongnk
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On pages whose
loadevent is delayed (slow external resources, network stalls,or intentionally hung requests), pressing
yyto copy the URL silently doesnothing: the URL is never written to the clipboard and the "Yanked" feedback
HUD never appears.
Root cause:
HUD.show(),HUD.showFindMode(),HUD.copyToClipboard(), andHUD.pasteFromClipboard()all callawait DomUtils.documentComplete(), whichwaits for the
loadevent. If that event is delayed, all HUD operations areblocked for the same duration.
Fix
Replace
documentComplete()(waits forload) withdocumentReady()(waitsfor
DOMContentLoaded) in all four methods.documentReady()already handles all readyState cases correctly: it resolvesimmediately when
document.readyState !== \"loading\"and only adds aDOMContentLoadedlistener when the page is still in the loading state.The HUD is a
chrome-extension://iframe overlay with its own independentlifecycle — it does not depend on the host page's external resources finishing.
showClipboardUnavailableMessageis intentionally left ondocumentComplete()since it is only emitted after a clipboard operation has already been attempted.
Testing
loadevent (e.g. one with a slow/hangingexternal resource, or use DevTools to throttle network after DOMContentLoaded).
yy— before this fix the HUD would stay silent until the page fullyloaded; after the fix the "Yanked" notification appears immediately and the
URL is in the clipboard.
yystill works normally on fully-loaded pages.