1397656 - Update tests within editor/ to comply with new toplevel data: URI navigation policy

Status: RESOLVED FIXED

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

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1397656_editor_tests.patch

Masayuki, 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 editor/ to fail:

* editor/libeditor/tests/browser_bug527935.js
* editor/libeditor/tests/test_bug635636.html
* editor/libeditor/tests/test_bug966155.html
* editor/libeditor/tests/test_bug966552.html

Please note that the only change I performed for those tests is to load them from an external file instead of using a data: URI.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #1)
> Masayuki, we are about to block toplevel data: URI navigations within
> Firefox (because those navigations are mostly only used for phishing
> attempts).

Oh, then, cannot load simple testcase as data URI even if inputting it in the URL bar even on Nightly??

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Comment on attachment 8905455 [details] [diff] [review]
bug_1397656_editor_tests.patch

># HG changeset patch
># User Christoph Kerschbaumer <[e-mail]>
># Date 1504781468 -7200
>#      Thu Sep 07 12:51:08 2017 +0200
># Node ID 5e11f845f965d8515e6a6b959f1b300ca14a989b
># Parent  575a7492d497a02c32f270a80a9356d6ab78e94b
>Bug 1397656 - Update tests within editor/ to comply with new toplevel data: URI navigation policy. r=masayuki
>
>diff --git a/editor/libeditor/tests/browser.ini b/editor/libeditor/tests/browser.ini
>--- a/editor/libeditor/tests/browser.ini
>+++ b/editor/libeditor/tests/browser.ini
>@@ -1,6 +1,8 @@
> [browser_bug527935.js]
> skip-if = toolkit == 'android'
>-support-files = bug527935.html
>+support-files =
>+  bug527935.html
>+  bug527935_2.html

I like "file_" or "window_" or "frame_" prefixed file name better. However, following test's support file doesn't have any prefix. I guess, browser chrome tests don't use such prefix... Okay, keep using it.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #2)
> Oh, then, cannot load simple testcase as data URI even if inputting it in
> the URL bar even on Nightly??

We only block navigations to data: URIs, e.g. through click, window.open, window.location. When inputting a data: URI directly in the URL bar it will continue to work.

Comment 5

[Pulsebot]

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

Comment 6

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

Backed out for failing modified mochitest test_bug966552.html:

https://hg.mozilla.org/integration/mozilla-inbound/rev/78ab7b444a9c125ead34fae6777021d8103571d7

First push with ran failing tests: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=6e4988e39120b0a1f7a7eeeb872c4781f9d5d9ee&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=retry&filter-resultStatus=testfailed&filter-resultStatus=busted
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=129301471&repo=mozilla-inbound

17:55:09     INFO -  1392 INFO TEST-START | editor/libeditor/tests/test_bug966552.html
17:55:10     INFO -  TEST-INFO | started process screenshot
17:55:10     INFO -  TEST-INFO | screenshot: exit 0
17:55:10    ERROR -  1393 INFO TEST-UNEXPECTED-FAIL | editor/libeditor/tests/test_bug966552.html | undefined assertion name - got "st\n", expected "st"
17:55:10     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:312:5
17:55:10     INFO -      runTest/<@editor/libeditor/tests/test_bug966552.html:40:5
17:55:10     INFO -      focusedOrLoaded/<@SimpleTest/SimpleTest.js:795:59

Comment 7

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

Failure only hits Windows.

Comment 8

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #6)
> Backed out for failing modified mochitest test_bug966552.html:

Thanks, I'll take a look.

Comment 9

[Christoph Kerschbaumer [:ckerschb]]

Since we are loading from an external file it seems that body.textContent includes the \n, hence calling .trim() before doing the comparison. Also, since we are loading the data: URI from an external file now, there is no need for setting
> ["security.data_uri.unique_opaque_origin", false]]}, nextTest);

I also removed that.
TRY looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fe2deb311fe28e7e43e0616ddf9368a1ce555d7&selectedJob=129589335

Comment 10

[Pulsebot]

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

Comment 11

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

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