Closed Bug 1397653 Opened 2 years ago Closed 2 years ago
Update tests within docshell/ to comply with new toplevel data: URI navigation policy
No description provided.
Priority: -- → P3
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
Attachment #8906370 - Flags: review?(bugs)
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Priority: P3 → P2
Whiteboard: [domsecurity-backlog1] → [domsecurity-active]
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.
Attachment #8906370 - Flags: review?(bugs) → review-
(In reply to Olli Pettay [:smaug] from comment #2) > You end up changing what is being tested. Or explain why that is not happening.
(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 on attachment 8906370 [details] [diff] [review] bug_1397653_docshell_tests.patch ok
Attachment #8906370 - Flags: review- → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2b64aee9198d Update tests within docshell/ to comply with new toplevel data: URI navigation policy. r=smaug
You need to log in before you can comment on or make changes to this bug.