1898321 - Text-Fragments: Deal correctly with changes to the DOM during page load

Status: RESOLVED FIXED

(Core :: DOM: Navigation, task)

Description

[Jan Jaeschke [:jjaschke]]

Currently, text fragments on this page won't be highlighted or scrolled to. The reason is that the main content of this page is kept as <script type="text/template">, which is ignored during parsing, and then converted into DOM nodes as a callback in DOMContentLoaded.

The current implementation clears all uninvoked text directives that are not found on the page in the first call to Document::ScrollToRef(). However, there are multiple calls to ScrollToRef() during page load (which this essentially the same as what is described in try to scroll to the fragment, step 2). Hence, only text directives which are found must be removed from the uninvoked text directives.

Additionally, after parsing has finished (which is step 2.1 in the beforementioned section of the spec) all uninvoked text directives need to be cleared, since loading of the document is done now. The right part for this seems to be after the call to document->ScrollToRef() in nsDocumentViewer::LoadComplete().

Comment 1

[Jan Jaeschke [:jjaschke]]

Attachment: Bug 1898321 - Text Fragments: Correctly find text fragments if there are DOM changes during page load. r=farre!,avandolder,#dom-core

Before this patch, the first call to Document::ScrollToRef() would attempt to find all text directives in the document, and, if found, highlight + scroll to them. Then, it would clear the list of uninvoked directives.

However, ScrollToRef() is called multiple times during a page load, and the DOM can change (e.g. if DOMContentLoaded does some changes).

This patch fixes this by only removing the text directives which were found in the document from the list of uninvoked directives.
This way, subsequent calls to ScrollToRef() would ensure that all text directives are highlighted, and the first text directive is scrolled to.
Additionally, all remaining uninvoked text directives at the end of a page load (ie. text directives that are not on the page) are cleared.

A test was added as well.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/279025d15551
Text Fragments: Correctly find text fragments if there are DOM changes during page load. r=farre,dom-core

Comment 3

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46756 for changes under testing/web-platform/tests

Comment 4

[Iulian Moraru]

Backed out 4 changesets (bug 1898321, bug 1897942, bug 1897956) for causing multiple failures.

• Push with failures

• Failure log for build bustages

• Failure log for crashtest failures

• Backout link

Comment 5

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/986a6317351d
Text Fragments: Correctly find text fragments if there are DOM changes during page load. r=farre,dom-core

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46832 for changes under testing/web-platform/tests

Comment 8

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/986a6317351d

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot