1397657 - Update tests within toolkit/ to comply with new toplevel data: URI navigation policy

Status: RESOLVED FIXED

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

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1397657_toolkit_tests.patch

Francois, we are about to block toplevel data: URI navigations within Firefox (because those navigations are mostly only used for phishing attempts). Flipping the pref |security.data_uri.block_toplevel_data_uri_navigations| causes a test within toolkit/ to fail:
* browser_page_change_print_original.js

Please note that I didn't add an empty line and the end for the two files on purpose, because the old test setup explicitly required a string comparison to make the test succeed - I didn't want to touch that.

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1397657_toolkit_tests.patch

Francois, apparently eslint is not amused by not having empty lines at the end of the file. Instead I am using trim() now, otherwise we face the line ending problem because textContent would include \n.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fe2deb311fe28e7e43e0616ddf9368a1ce555d7

Comment 3

[François Marier [:francois]]

Comment on attachment 8906003 [details] [diff] [review]
bug_1397657_toolkit_tests.patch

Review of attachment 8906003 [details] [diff] [review]:
-----------------------------------------------------------------

That looks fine to me, but it may be good to have someone who actually knows about this test reviewing your change.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1397657_toolkit_tests.patch

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8906348 [details] [diff] [review]
bug_1397657_toolkit_tests.patch

Dave, let me summarize the test changes here real quick:
We are about to block toplevel data: URI navigations within Firefox (because those navigations are mostly only used for phishing attempts). Flipping the pref |security.data_uri.block_toplevel_data_uri_navigations| causes the following tests within toolkit/ to fail:
* browser_page_change_print_original.js
For this test I am loading the data: URI from an external file. Please note that I have to use trim() because otherwise we are facing a line ending problem because textContent would include the \n.

* toolkit/mozapps/extensions/test/xpinstall/browser_datauri.js
Since Bug 1394554 we are also blocking redirects to data: URIs. I looked at browser_datauri.js and there is this comment on top "Checks that a chained redirect through a data URI and javascript is blocked", hence I think it makes sense to explicitly flip the pref to false, so this test keeps testing what it's supposed to be testing.

Are you fine with those changes?

Comment 6

[Dave Townsend [:mossop]]

Comment on attachment 8906348 [details] [diff] [review]
bug_1397657_toolkit_tests.patch

Review of attachment 8906348 [details] [diff] [review]:
-----------------------------------------------------------------

Sounds fine.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff02238411b7
Update tests within toolkit/ to comply with new toplevel data: URI navigation policy. r=francois,dtownsend

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/ff02238411b7