1860915 - Implement part of Document Policy for force-load-at-top for text fragment

Status: NEW

(Core :: DOM: Core & HTML, enhancement)

Description

[Andrew McCreight [:mccr8]]

The scroll-to-text-fragment defines a "configuration point" for Document Policy, which is a boolean force-load-at-top. The idea is that a document can add this to their HTTP header and block scroll-to-text-fragment. This header can also, for some reason, block plain old element fragment scrolling.

I've implemented enough of this so that the force-load-at-top.html can pass, though I'm not entirely sure why the load from history scroll restoration part of the text passes, as I have not actually modified anything in session restore.

This patch does not implement the policy iframe attribute. This lets a outer page require a kind of minimum Document Policy from the inner page, via a special Sec-Required-Document-Policy in the request, and then the load gets blocked if the response from the server doesn't comply. While this feature makes sense for some Document Policy things that are requiring features to ensure performance, I feel like it doesn't make any sense for force-load-at-top.

Anyways, this chunk of code I've implemented doesn't exist as a spec outside of scroll-to-text-fragment so I'm not sure what to do with it, but I figured I'd file a bug and add a patch so Jan or whomever can take a look.

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: Bug 1860915, part 1 - Implement part of Document Policy for force-load-at-top.

Comment 2

[Andrew McCreight [:mccr8]]

I should also dig around to other places where CSP is initialized (like where InitCSP is called) and see if I need to add in my "document policy support" there, too. But hey, tests pass.

Comment 3

[Eric Lawrence (@ericlaw)]

This directive would be useful to address cross-window gesture jacking attacks whereby an attacker can trick a user into invoking a button in a cross-origin document.

See https://www.paulosyibelo.com/2024/02/cross-window-forgery-web-attack-vector.html

Comment 4

[Andrew McCreight [:mccr8]]

I've been looking at this. Peter helped a bit trying to figure out where the check should go. Although it looks like jjaschke already added a note that the check should get added to Document::ScrollToRef() (which is one of the places Peter mentioned to me). If I return from there if ForceLoadAtTop() is true, then all of the force-load-at-top tests pass. I'm not sure if that's a good sign or a bad sign. The spec changed from blocking scroll on load for element fragment for same origin navigation to not doing it and they didn't add any tests. I guess I should look into what the tests are actually doing. Peter also had some questions about how this blocking would interact with BFCache and I don't know if that is accounted for in these tests.

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: Bug 1860915, part 2 - Return from Document::ScrollToRef() if force-load-at-top is set.

Comment 6

[Andrew McCreight [:mccr8]]

I'm not sure if this strictly depends on bug 1897956 and bug 1898321, but bug 1898630 does and ideally we'd land them around the same time so I'm marking it like that.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c93c2e13371c
part 1 - Implement part of Document Policy for force-load-at-top. r=necko-reviewers,smaug,kershaw
https://hg.mozilla.org/integration/autoland/rev/2c0b981cfd34
part 2 - Return from Document::ScrollToRef() if force-load-at-top is set. r=jjaschke,smaug

Comment 8

[Cristian Tuns]

Backed out for blocking the backout of Bug 1888756

• Backout link

Comment 9

[Florin Bilt]

Hello !
The following alert was generated by the backout . Do you think this bug is responsible for generating the alert ?

Perfherder has detected a browsertime performance change from push 5f6f41a83f0ea8d64264cf04f49a5940f9c8705b.

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
22%	wikipedia fcp	android-hw-a51-11-0-aarch64-shippable-qr	warm webrender	69.60 -> 85.21	Before/After
11%	speedometer3 TodoMVC-Preact-Complex-DOM/Adding100Items/Sync	macosx1400-64-shippable-qr	fission webrender	0.70 -> 0.78	Before/After
8%	speedometer3 TodoMVC-Svelte-Complex-DOM/Adding100Items/Sync	macosx1400-64-shippable-qr	fission webrender	1.51 -> 1.62	Before/After
7%	speedometer3 TodoMVC-Svelte-Complex-DOM/Adding100Items/Sync	windows10-64-shippable-qr	fission webrender	3.21 -> 3.43	Before/After
6%	speedometer3 TodoMVC-Svelte-Complex-DOM/Adding100Items/Sync	windows10-64-nightlyasrelease-qr	fission webrender	3.17 -> 3.37	Before/After
5%	speedometer3 TodoMVC-Preact-Complex-DOM/Adding100Items/total	macosx1400-64-shippable-qr	fission webrender	5.19 -> 5.44	Before/After
4%	speedometer3 TodoMVC-Svelte-Complex-DOM/CompletingAllItems/total	windows10-64-shippable-qr	fission webrender	7.37 -> 7.69	Before/After
4%	speedometer3 Charts-chartjs/Show tooltip/Sync	macosx1400-64-shippable-qr	fission webrender	0.08 -> 0.08	Before/After
4%	speedometer3 TodoMVC-Svelte-Complex-DOM/CompletingAllItems/Async	windows10-64-shippable-qr	fission webrender	5.97 -> 6.18	Before/After
3%	speedometer3 TodoMVC-Preact-Complex-DOM/total	macosx1400-64-shippable-qr	fission webrender	11.59 -> 11.93	Before/After
2%	speedometer3 TodoMVC-Preact-Complex-DOM/DeletingAllItems/Async	macosx1400-64-shippable-qr	fission webrender	1.74 -> 1.78	Before/After

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
17%	fandom largestContentfulPaint	linux1804-64-shippable-qr	fission warm webrender	47.46 -> 39.44	Before/After
9%	speedometer3 TodoMVC-Svelte-Complex-DOM/Adding100Items/Async	windows10-64-nightlyasrelease-qr	fission webrender	9.35 -> 8.52	Before/After
8%	wikia FirstVisualChange	linux1804-64-shippable-qr	cold fission webrender	330.15 -> 305.37	Before/After
7%	speedometer3 TodoMVC-Svelte-Complex-DOM/Adding100Items/Async	windows10-64-shippable-qr	fission webrender	9.36 -> 8.67	Before/After
7%	wikia largestContentfulPaint	linux1804-64-shippable-qr	fission warm webrender	193.05 -> 180.32	Before/After
...	...	...	...	...	...
2%	speedometer3 Charts-observable-plot/Stacked by 6/Async	macosx1400-64-shippable-qr	fission webrender	2.38 -> 2.33	Before/After

As author of one of the patches included in that push, we need your help to address this regression.
Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests. Please follow our guide to handling regression bugs and let us know your plans within 3 business days, or the patch(es) may be backed out in accordance with our regression policy.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 899

For more information on performance sheriffing please see our FAQ.

Comment 10

[Andrew McCreight [:mccr8]]

The code I landed is behind a preference, so the only change should be a single preference check, so it shouldn't affect performance.