Skip to content

Commit

Permalink
Revert "[text-fragment] Attempt search for dynamic content"
Browse files Browse the repository at this point in the history
This reverts commit 6bd222a5ff1b875ac739df526a59799ded6b6676.

Reason for revert: Likely causing recent builder failures.
Example failure: https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20Tests/131991/overview
Failing: external/wpt/scroll-to-text-fragment/iframes.sub.html

Original change's description:
> [text-fragment] Attempt search for dynamic content
>
> Text fragments currently perform up to two searches, one when document
> parsing completes and a second attempt when document load completes (if
> load wasn't completed at parse complete time). However, pages, often
> load content after document load, when content is "dynamically loaded".
> One popular example is Mobile Wikipedia, which adds `hidden=until-found`
> on collapsed sections in idle tasks after load. This meant
> text-fragments couldn't target pages like these.
>
> This CL attempts to make text fragments work on dynamically loaded pages
> by performing a third attempt, if needed. If all directives haven't
> matched at load time, a delayed task is scheduled for 500ms that will
> request attachment on unmatched directives. TextFragmentAnchor listens
> for relevant changes in the DOM and reschedules this task each time a
> change is made. At 3000ms it gives up and performs the search to avoid
> waiting forever.
>
> At a high level, this CL tries to separate the state tracking of actions
> performed for the first matching directive from the state tracking for
> running multiple searches. This is done by introducing a new iteration_
> enum tracking the latter.
>
> Change-Id: Ie01012935152b12f2d724469010128c7dd34afb8
> Bug: 963045
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4586061
> Reviewed-by: Vladimir Levin <[email protected]>
> Commit-Queue: David Bokan <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1160040}

Bug: 963045
Change-Id: I64babfbb5ac1849f3ea3c7bf3fdb41b12390b657
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4629649
Bot-Commit: Rubber Stamper <[email protected]>
Owners-Override: thefrog <[email protected]>
Commit-Queue: thefrog <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1160118}
  • Loading branch information
thefrog authored and chromium-wpt-export-bot committed Jun 20, 2023
1 parent 03365e3 commit 5cd3700
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 8 deletions.
4 changes: 0 additions & 4 deletions lint.ignore
Original file line number Diff line number Diff line change
Expand Up @@ -679,10 +679,6 @@ DUPLICATE-BASENAME-PATH: svg/struct/reftests/reference/green-100x100.svg

SET TIMEOUT: mediacapture-insertable-streams/MediaStreamTrackProcessor-video.https.html

# This is a subresource which cannot use step_timeout without becoming a test
# itself. See https://github.com/web-platform-tests/wpt/issues/16933
SET TIMEOUT: scroll-to-text-fragment/iframe-target.html

# Ported crashtests from Mozilla
SET TIMEOUT: editing/crashtests/backcolor-in-nested-editing-host-td-from-DOMAttrModified.html
SET TIMEOUT: editing/crashtests/contenteditable-will-be-blurred-by-focus-event-listener.html
Expand Down
9 changes: 5 additions & 4 deletions scroll-to-text-fragment/iframe-target.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@

window.addEventListener('message', (e) => {
if (e.data == 'getResult') {
// Use a timeout to get results - in the elementId fallback case, the
// browser may retry the text fragment search a few times before giving
// up and trying the elementid.
setTimeout(postResult, 2000);
// rAF twice in case there is any asynchronicity in scrolling to the
// target.
window.requestAnimationFrame(() => {
window.requestAnimationFrame(postResult);
})
} else if (e.data == 'reset') {
window.location.hash = '';
window.scrollTo(0, 0);
Expand Down

0 comments on commit 5cd3700

Please sign in to comment.