1397653 - Update tests within docshell/ to comply with new toplevel data: URI navigation policy

Status: RESOLVED FIXED

(Core :: DOM: Security, enhancement, P2)

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1397653_docshell_tests.patch

Smaug, overall we are facing several failing tests within docshell/ when flipping the pref |security.data_uri.block_toplevel_data_uri_navigations|:
* docshell/test/navigation/test_bug13871.html
* docshell/test/navigation/test_bug270414.html
* docshell/test/navigation/test_bug278916.html
* docshell/test/navigation/test_bug279495.html
* docshell/test/navigation/test_bug430624.html
* docshell/test/navigation/test_not-opener.html
* docshell/test/navigation/test_not-opener.html
* docshell/test/navigation/test_sessionhistory.html
* docshell/test/navigation/test_triggeringprincipal_window_open.html
* docshell/test/test_bug598895.html
* docshell/test/test_bug637644.html
* docshell/base/crashtests/914521.html

Most of the failing tests within docshell/test/navigation rely on NavigationUtils.js. Now that we are loading the data: URI from an external file .textContent as well as .innerHTML include the \n, hence I use .trim() before performing the string comparision.

The test file_scrollRestoration.html is not really relying on the data: URI itself, hence I suppose it's fine to just use about:blank instead of using an external file.

The test test_triggeringprincipal_window_open.html is using window.open("data:", since that can not happen anymore, I suppose it's fine to remove that test. If you prefer, I am also happy to use setPushPrefEnv and set the pref to false for that test.

For thest test crashtests/914521.html, we already set the pref security.data_uri.unique_opaque_origin, hence I think it makes sense to also explicity set the pref for this test.

The test chrome/test_bug364461.xul uses several data: URIs, I suppose it's fine to keep the test as is and just push the pref explicitly for the test, if you prefer, I am happy to load those data: URIs from an external file.

Everything else should be self explanatory, if not, please let me know!

Seems there are no more docshell tests that need to be rewritten:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb517e30d8209331f929f9a3f9fd2b61a5fed63c

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8906370 [details] [diff] [review]
bug_1397653_docshell_tests.patch

Hmm, Why is using about:blank in file_scrollRestoration.html ok?
http://searchfox.org/mozilla-central/rev/70cfd6ceecacbe779456654b596bbee4f2b8890b/docshell/base/nsDocShell.cpp#12461,12481

You end up changing what is being tested.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Olli Pettay [:smaug] from comment #2)
> You end up changing what is being tested.

Or explain why that is not happening.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Olli Pettay [:smaug] from comment #3)
> Or explain why that is not happening.

When looking at the test I thought it just needs some page to navigate away from the current. In my opinion it doesn't really matter that page that is. Then using history.back() should restore the setting that were used on that previous page. Isn't that what the test is doing? If not, then I am happy to rewrite those bits to load some external foo page.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8906370 [details] [diff] [review]
bug_1397653_docshell_tests.patch

ok

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b64aee9198d
Update tests within docshell/ to comply with new toplevel data: URI navigation policy. r=smaug

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2b64aee9198d