1389251 - data: URIs with spaces around MIME type are rejected in Firefox but not in other browsers

Status: VERIFIED FIXED

(Core :: Networking, defect, P2)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Load attached testcase -- or just load this data URI:
     data: text/html,<h1>Large text 
(Edge won't accept data URIs in the URL bar -- hence, including a testcase with an iframe as well, for Edge testability.)

EXPECTED RESULTS:
Browser should render the HTML, showing "Large text".

ACTUAL RESULTS:
Browser doesn't render the HTML -- it treats it as having an unrecognized media type, I think, which prompts a file download.

This is a result of the space character between "data:" and "text/html".  I think we might be correct according to https://tools.ietf.org/html/rfc2397#section-3 but it seems other browsers disagree with us, so from a practical perspective we probably need to align with the other browsers' behavior.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: reference case 1 (with space removed)

Comment 3

[Boris Zbarsky [:bzbarsky]]

We really need a sane spec for data: parsing.

Comment 4

[Daniel Holbert [:dholbert]]

(Note that we also have related bug 1104311 and bug 1388594, but it looks to me like those are about whitespace in the payload -- i.e. the content after the "," -- rather than whitespace in the media type.)

Comment 5

[Daniel Holbert [:dholbert]]

These browsers all give "EXPECTED RESULTS" (rendering the "Large text" html in the attached testcase):
 Chrome 60   (Mac)
 Safari 10.1 (Mac)
 Edge 15     (Win10)
 Opera 46    (Mac)
 Opera 12.15 (Mac)

Only Firefox gives "ACTUAL RESULTS".

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Boris Zbarsky [:bz] (vacation Aug 14-27) from comment #3)
> We really need a sane spec for data: parsing.

True -- but for this proximal issue at least, it seems like we should strip whitespace between the colon and the media type.

Just testing in Chrome, it seems like that's what they do, at least. (That's the only place they're permissive -- spaces that occur *within* the media type are preserved and break the rendering -- e.g. these are all treated as non-HTML, and match what happens if I provide a bogus media type:
 data:t ext/html,<h1>Large text
 data:text/ html,<h1>Large text
 data:text/h tml,<h1>Large text

Comment 7

[Daniel Holbert [:dholbert]]

I'm going to assign this to kahs to work on when he's done with the bug he's on right now.

I'm not 100% sure, but I think the work here will be in:
https://dxr.mozilla.org/mozilla-central/rev/4c5fbf49376351679dcc49f4cff26c3c2e055ccc/netwerk/protocol/data/nsDataHandler.cpp#157

Comment 8

[Daniel Holbert [:dholbert]]

Actually I guess we already have parsed |contentType| at that point, so it'd be a bit earlier.

Comment 9

[Boris Zbarsky [:bzbarsky]]

What about:

  data:text/html ,<h1>Large text

?  If the handle that as HTML, I guess the point is that we'd just want to trim whitespace around our tentative media type.

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: testcase 2

Comment 11

[Daniel Holbert [:dholbert]]

(In reply to Boris Zbarsky [:bz] (vacation Aug 14-27) from comment #9)
> What about:
> 
>   data:text/html ,<h1>Large text

Good question. Chrome and Edge do treat that as HTML, yes. (I've encoded that in "testcase 2" for testing in Edge.)

> If the handle that as HTML, I guess the point is that we'd just want to
> trim whitespace around our tentative media type.

Yeah, that makes sense to me.

Comment 12

[Anne (:annevk)]

FWIW, I started writing some tests to have something to back an eventual specification and it doesn't seem like Chrome strips all whitespace. U+0009, U+000A, and U+000D are already gone due to the URL parser. U+000C is left in by Chrome and U+0020 is stripped. So e.g.

  data:%0Ctext/html,x

ends up with a Content-Type of "%0ctext/html" (which results in a download when you navigate to it). Same happens (including the Content-Type value, seems they percent encode stuff) for

  data:\x0Ctext/html,x

assuming you replace \x0C with a U+000C literal.

If you're interested in my tests I posted a copy at https://gist.github.com/annevk/4287452653921b2b7de35e4208b4a985 and they can be run using web-platform-tests (at same URL with .js replaced with .html).

Anything worth standardizing can be noted here: https://github.com/whatwg/fetch/issues/234.

Comment 13

[Anne (:annevk)]

(By the way, one thing that would be nice to fix is to use "text/plain;charset=US-ASCII" as fallback for unrecognized MIME types rather than "text/plain". We already use that as default MIME type and there's no real reason for them to be different I think. US-ASCII is a little dated and actually ends up meaning windows-1252, but whatever, it's interoperable.)

Comment 14

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #7)
> I'm going to assign this to kahs to work on when he's done with the bug he's
> on right now.

(Cancel that - he's gonna work on some ruby bugs instead. Unassigning -- feel free to take, though maybe it would be wise to wait to see how the standardizing shakes out.)

Comment 15

[u408661]

Sticking this in next for now (to keep it on our radar), but based on comment 14 we may want to hold off on doing any real work until the spec solidifies?

Comment 16

[Anne (:annevk)]

https://github.com/whatwg/fetch/pull/579 has my work in progress on a better specification.

Given the tests I've written thus far I think that fixing this by trimming leading and trailing whitespace from the MIME type portion is fine.

There's more work to be done on data: URL processing, but we can do it incrementally (unless there's a good reason not to that I don't know about).

Comment 17

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 18

[Daniel Holbert [:dholbert]]

Attachment: testcase 3 (with image/svg+xml mime type)

Comment 19

[Daniel Holbert [:dholbert]]

BTW, it seems like we get this right IF the data URI has ";base64" after the mimetype, as shown at e.g. https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=5761

(I believe that means "data: image/svg+xml;base64,..." would work, even though "data: image/svg+xml,..." is broken per my most recent attachment.  I can't test the ;base64 version with a useful URL at the moment, since data URI kitchen (my preferred base64-data-URI-encoder) seems to not be working for me right now.)

Comment 20

[Anne (:annevk)]

Note: https://software.hixie.ch/utilities/cgi/data/data is broken by design. We no longer support redirects to data: URLs.

Comment 21

[Boris Zbarsky [:bzbarsky]]

This is silly.  We already have code that tries to strip whitespace from the content type.  It's just not working right because it runs prior to unescaping, so at that point the whitespace is %20, not ' '.  This was probably missed in the patches for bug 391951.

That said, it seems to me that https://fetch.spec.whatwg.org/#data-urls step 6 would have the same problem, unless the URL spec allows unescaped literal ' ' in URLs?  Which maybe it does in this case because https://url.spec.whatwg.org/#cannot-be-a-base-url-path-state uses https://url.spec.whatwg.org/#c0-control-percent-encode-set which does not include U+0020.  This is unlike https://url.spec.whatwg.org/#path-state which uses https://url.spec.whatwg.org/#path-percent-encode-set which _does_ include U+0020.  In our case, I think we don't really have this concept of "cannot be a base URL path state".

We could try to hack around this in the data handler, but do we have an ETA on having our URL parsing just match the URL spec?

Comment 22

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=73456fceda31259e2f49066e1d1b6fb1d9e7e68f

Comment 23

[Valentin Gosu [:valentin] (he/him)]

So, the main problem here is that while nsDataHandler::ParsePathWithoutRef does strip the whitespace from the content type, it doesn't change the original spec, so the URI we end up creating will still percent encode the space, without stripping it.

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #21)
> We could try to hack around this in the data handler, but do we have an ETA
> on having our URL parsing just match the URL spec?

No. I'll try and dedicate more time to this, but it'll still be quite a long process. The assumptions our internal code makes about URIs usually create roadblocks in this undertaking.

Comment 24

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec6ed81336f6fd7a603455f72892afa0f194ac52

Comment 25

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f8c1861209d8d9f6a7b6b365f569d4bb87e7e942

Comment 26

[Valentin Gosu [:valentin] (he/him)]

So, I tried to make ParsePathWithoutRef strip whitespace from the initial URI MIME type. Apart from the issue that it actually changed the URI, it turned out to be much more complicated than I expected.
So I looked more carefully into :bz's suggestion, and _obviously_ it was a much better fix to the problem. (/me makes mental note for the future: always try Bz's solution first)

The only issue I've got now is that browser_webconsole_context_menu_copy_entire_message.js is failing, but I will ask jdescottes to help me with it.

Now I just need to audit a few other calls to NS_EscapeURL(esc_OnlyNonASCII), to make sure spaces aren't an issue.

Comment 27

[Valentin Gosu [:valentin] (he/him)]

Attachment: Do not escape spaces in nsSimpleURI

This change makes nsEscape::T_EscapeURL not escape spaces when passed esc_OnlyNonASCII.
This fixes a web-compat issue for URLs such as "javascript: alert('hello');" and the fact that data: URIs with spaces around MIME type are rejected.

Comment 28

[Valentin Gosu [:valentin] (he/him)]

Hi Julian,

I was wondering if you could help me with this failure in browser_webconsole_context_menu_copy_entire_message.js:
https://treeherder.mozilla.org/logviewer.html#?job_id=181731693&repo=try&lineNumber=10729

The patch above makes data: URIs no longer encode spaces, which I guess affects this test because it loads a data URI.
I can get the test to pass if I remove all the "The last line is an empty new line" checks.
Can you help me figure out if the new behaviour is correct, or otherwise what changes need to be made?

I used this patch to figure out what is different:
https://pastebin.mozilla.org/9087038
And got these outputs:
before: https://pastebin.mozilla.org/9087039
after: https://pastebin.mozilla.org/9087040

So whereas before the clipboard text was
```17:26:32.299 simple text message data:<script>  window.logStuff = function () {    console.log("simple text m:1:38
```

It is now:
```17:32:04.213 console.trace() data:<script>  window.logStuff = function () {    console.log("simple text message");    function wr:1:103
  wrapper data:text/html;charset=utf-8,<script>  window.logStuff = function () {    console.log("simple text message");    function wrapper() {      console.trace();    }    wrapper();  };</script>:1:103
  window.logStuff data:text/html;charset=utf-8,<script>  window.logStuff = function () {    console.log("simple text message");    function wrapper() {      console.trace();    }    wrapper();  };</script>:1:128
```

Thanks!

Comment 29

[Julian Descottes [:jdescottes]]

I don't think your patch breaks the behavior, it's just that this test is badly designed.

So the first issue is that a lot of asserts in this test are wrong:
  ok(lines.length, 2, "There are 2 lines in the copied text");

The intent was to check if lines.length was equal to 2. We should have used is(lines.length, 2, ...). Sorry about that.

Now the real issue is that with your patch, the test clicks on the second message rather than on the first. 

If you look at the "location" of the messages, before your patch we have
  data:<script>  window.logStuff = function () {    console.log("simple text m:1:38
And after your patch
  data:<script>  window.logStuff = function () {    console.log("simple text message"); function wr:1:38

(logic to shorten source locations is at https://searchfox.org/mozilla-central/rev/3737701cfab93ccea04c0e9cab211ad10f931d87/devtools/client/shared/source-utils.js#117-184 , now that spaces are no longer escaped, more "text" is displayed)

And in our test, we find messages (via findMessage()) by matching textContent. For the first message we look for "simple text message", which is now part of the origin for both the first and the second message. 

To fix this, several options: 

1/ pass a selector to findMessage() which will force it to look only in the text of the messages:
  let message = 
    await waitFor(() => findMessage(hud, "simple text message", ".message-body"));
  message = message.closest(".message");
( we still need to go back to the .message to copy the expected string)

https://pastebin.mozilla.org/9087101

2/ modify the test source to avoid having "simple text message" in the location, eg console.log("simple " + "text message");

Let me know if this fixes the issue.

Comment 30

[Valentin Gosu [:valentin] (he/him)]

(In reply to Julian Descottes [:jdescottes][:julian] from comment #29)
> 2/ modify the test source to avoid having "simple text message" in the
> location, eg console.log("simple " + "text message");
> 
> Let me know if this fixes the issue.

Thanks a lot! This last solution fixed it.
I also updated the test to use is(lines.length...) instead of ok()

Comment 31

[Julian Descottes [:jdescottes]]

(In reply to Valentin Gosu [:valentin] from comment #30)
> (In reply to Julian Descottes [:jdescottes][:julian] from comment #29)
> > 2/ modify the test source to avoid having "simple text message" in the
> > location, eg console.log("simple " + "text message");
> > 
> > Let me know if this fixes the issue.
> 
> Thanks a lot! This last solution fixed it.
> I also updated the test to use is(lines.length...) instead of ok()

Thanks for cleaning this up! FYI, I am fixing other similar issues in Bug 1467407.

Comment 32

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1389251 - Do not escape spaces in nsSimpleURI

This change makes nsEscape::T_EscapeURL not escape spaces when passed esc_OnlyNonASCII.
This fixes a web-compat issue for URLs such as "javascript: alert('hello');" and the fact that data: URIs with spaces around MIME type are rejected.

Review commit: https://reviewboard.mozilla.org/r/250004/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250004/

Comment 33

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1389251 - Add esc_Spaces that may be used to force escaping of spaces

See Bug 1389251 comment 29 for full explanation of why we do this.
Also replaces ok(lines.length, value) with is(lines.length, value)

Review commit: https://reviewboard.mozilla.org/r/250006/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250006/

Comment 34

[Julian Descottes [:jdescottes]]

Comment on attachment 8984204 [details]
Bug 1389251 - Add esc_Spaces that may be used to force escaping of spaces

https://reviewboard.mozilla.org/r/250006/#review256316

Looks good to me, thanks!

Comment 35

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8984203 [details]
Bug 1389251 - Do not escape spaces in nsSimpleURI

https://reviewboard.mozilla.org/r/250004/#review256808

Sorry for the lag here....

I have a few questions/concerns:

1)  Do all the current consumers of esc_OnlyNonASCII (including the HTTP ones) want the new behavior?

2)  Does the change the behavior of data: URIs that _do_ have %20 in them?  I'd assume not, but wanted ot check.

I'm a little surprised there aren't other URL constructor tests in wpt for this "space allowed in some URIs but not others" thing....

Comment 36

[Valentin Gosu [:valentin] (he/him)]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #35)
> Comment on attachment 8984203 [details]
> Bug 1389251 - Do not escape spaces in nsSimpleURI
> 
> https://reviewboard.mozilla.org/r/250004/#review256808
> 
> Sorry for the lag here....
> 
> I have a few questions/concerns:
> 
> 1)  Do all the current consumers of esc_OnlyNonASCII (including the HTTP
> ones) want the new behavior?

I went through all of them - the HTTP redirect ones for example then build a HTTP URL with the path, so it gets properly escaped afterwards. It might be better to escape it anyway, just in case we change stuff in the future, or if there are code paths I'm missing.

I added an esc_Spaces flag that forces space escape. I use it for all calls where we _might_ want to escape spaces. Patch incoming.

> 2)  Does the change the behavior of data: URIs that _do_ have %20 in them? 
> I'd assume not, but wanted ot check.

It doesn't. I added some tests.

> I'm a little surprised there aren't other URL constructor tests in wpt for
> this "space allowed in some URIs but not others" thing....

It seems one more was recently added. :) I agree that there are quite few.

Comment 37

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1389251 - Fix browser_webconsole_context_menu_copy_entire_message.js so findMessage doesn't match on the wrong thing

See Bug 1389251 comment 29 for full explanation of why we do this.
Also replaces ok(lines.length, value) with is(lines.length, value)

Review commit: https://reviewboard.mozilla.org/r/250716/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250716/

Comment 38

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8984203 [details]
Bug 1389251 - Do not escape spaces in nsSimpleURI

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250004/diff/1-2/

Comment 39

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8984204 [details]
Bug 1389251 - Add esc_Spaces that may be used to force escaping of spaces

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250006/diff/1-2/

Comment 40

[Julian Descottes [:jdescottes]]

Comment on attachment 8984920 [details]
Bug 1389251 - Fix browser_webconsole_context_menu_copy_entire_message.js so findMessage doesn't match on the wrong thing

https://reviewboard.mozilla.org/r/250716/#review257204

Comment 41

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8984203 [details]
Bug 1389251 - Do not escape spaces in nsSimpleURI

https://reviewboard.mozilla.org/r/250004/#review257324

Comment 42

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8984204 [details]
Bug 1389251 - Add esc_Spaces that may be used to force escaping of spaces

https://reviewboard.mozilla.org/r/250006/#review257328

::: netwerk/base/nsSimpleURI.cpp:515
(Diff revision 2)
>  // string (and will result in .spec and .path having a terminal #).
>  nsresult
>  nsSimpleURI::SetRef(const nsACString &aRef)
>  {
>      nsAutoCString ref;
> -    nsresult rv = NS_EscapeURL(aRef, esc_OnlyNonASCII, ref, fallible);
> +    nsresult rv = NS_EscapeURL(aRef, esc_OnlyNonASCII | esc_Spaces, ref, fallible);

Are there web platform tests for this?  If not, should there be?  Does the new behavior match the other browsers?

Comment 43

[Valentin Gosu [:valentin] (he/him)]

(In reply to Boris Zbarsky [:bz] (Vacation Jun 16-24) (no decent commit message means r-) from comment #42)
> Comment on attachment 8984204 [details]
> Bug 1389251 - Add esc_Spaces that may be used to force escaping of spaces
> 
> https://reviewboard.mozilla.org/r/250006/#review257328
> 
> ::: netwerk/base/nsSimpleURI.cpp:515
> (Diff revision 2)
> >  // string (and will result in .spec and .path having a terminal #).
> >  nsresult
> >  nsSimpleURI::SetRef(const nsACString &aRef)
> >  {
> >      nsAutoCString ref;
> > -    nsresult rv = NS_EscapeURL(aRef, esc_OnlyNonASCII, ref, fallible);
> > +    nsresult rv = NS_EscapeURL(aRef, esc_OnlyNonASCII | esc_Spaces, ref, fallible);
> 
> Are there web platform tests for this?  If not, should there be?  Does the
> new behavior match the other browsers?

Just a couple, but it's good enough for now. We match Chrome - I haven't tested with the others.
https://searchfox.org/mozilla-central/rev/42930ab9634ebf3f62aed60f7d1c1bf25c0bf00c/testing/web-platform/tests/url/resources/urltestdata.json#153,168

Comment 44

[Boris Zbarsky [:bzbarsky]]

> Just a couple, but it's good enough for now.

OK.  So those tests fail with just the first patch?

Comment 45

[Valentin Gosu [:valentin] (he/him)]

(In reply to Boris Zbarsky [:bz] (Vacation Jun 16-24) (no decent commit message means r-) from comment #44)
> > Just a couple, but it's good enough for now.
> 
> OK.  So those tests fail with just the first patch?

Yes.

Comment 46

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6e89a11ae28e
Do not escape spaces in nsSimpleURI r=bz
https://hg.mozilla.org/integration/autoland/rev/c6baebf7b34c
Add esc_Spaces that may be used to force escaping of spaces r=bz,jdescottes
https://hg.mozilla.org/integration/autoland/rev/a76a3251a9d2
Fix browser_webconsole_context_menu_copy_entire_message.js so findMessage doesn't match on the wrong thing r=jdescottes

Comment 47

[Dorel Luca [:dluca]]

Backed out 3 changesets (bug 1389251) for browser-chrome failure on browser/base/content/test/pageinfo/browser_pageinfo_image_info.js. CLOSED TREE 

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=183288028&repo=autoland&lineNumber=1082

03:03:50     INFO -  6 INFO TEST-OK | browser/base/content/test/pageinfo/browser_pageinfo_firstPartyIsolation.js | took 592ms
03:03:50     INFO -  7 INFO checking window state
03:03:50     INFO -  8 INFO TEST-START | browser/base/content/test/pageinfo/browser_pageinfo_image_info.js
03:04:35     INFO -  TEST-INFO | started process screenshot
03:04:35     INFO -  TEST-INFO | screenshot: exit 0
03:04:35     INFO -  Buffered messages logged at 03:03:50
03:04:35     INFO -  9 INFO Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "data:text/html,<style type='text/css'>%23test-image,%23not-test-image {background-image: url('about:logo?c');}</style><img src='about:logo?b' height=300 width=350 alt=2 id='not-test-image'><img src='about:logo?b' height=300 width=350 alt=2><img src='about:logo?a' height=200 width=250><img src='about:logo?b' height=200 width=250 alt=1><img src='about:logo?b' height=100 width=150 alt=2 id='test-image'>" line: 0}]
03:04:35     INFO -  Buffered messages finished
03:04:35    ERROR -  10 INFO TEST-UNEXPECTED-FAIL | browser/base/content/test/pageinfo/browser_pageinfo_image_info.js | Test timed out -
03:04:35     INFO -  GECKO(1016) | MEMORY STAT | vsize 701MB | vsizeMaxContiguous 626MB | residentFast 196MB | heapAllocated 75MB
03:04:35     INFO -  11 INFO TEST-OK | browser/base/content/test/pageinfo/browser_pageinfo_image_info.js | took 45027ms
03:04:35     INFO -  Not taking screenshot here: see the one that was previously logged
03:04:35    ERROR -  12 INFO TEST-UNEXPECTED-FAIL | browser/base/content/test/pageinfo/browser_pageinfo_image_info.js | Found a tab after previous test timed out: data:text/html,<style type='text/css'>%23test-image,%23not-test-image {background-image: url('about:logo?c');}</style><img src='about:logo?b' height=300 width=350 alt=2 id='not-test-image'><img src='about:logo?b' height=300 width=350 alt=2><img src='about:logo?a' height=200 width=250><img src='about:logo?b' height=200 width=250 alt=1><img src='about:logo?b' height=100 width=150 alt=2 id='test-image'> -
03:04:35     INFO -  13 INFO checking window state
03:04:35     INFO -  14 INFO TEST-START | browser/base/content/test/pageinfo/browser_pageinfo_images.js
03:04:35     INFO -  GECKO(1016) | MEMORY STAT | vsize 704MB | vsizeMaxContiguous 626MB | residentFast 203MB | heapAllocated 78MB
03:04:35     INFO -  15 INFO TEST-OK | browser/base/content/test/pageinfo/browser_pageinfo_images.js | took 223ms
03:04:35     INFO -  16 INFO checking window state
03:04:35     INFO -  17 INFO TEST-START | browser/base/content/test/pageinfo/browser_pageinfo_permissions.js
03:04:37     INFO -  GECKO(1016) | MEMORY STAT | vsize 715MB | vsizeMaxContiguous 626MB | residentFast 214MB | heapAllocated 84MB
03:04:37     INFO -  18 INFO TEST-OK | browser/base/content/test/pageinfo/browser_pageinfo_permissions.js | took 1530ms
03:04:37     INFO -  19 INFO checking window state

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=a76a3251a9d242186b5de495e4388e56390b8fc8&filter-searchStr=browser%20chrome&selectedJob=183288091

Backout:
https://hg.mozilla.org/integration/autoland/rev/9ca74dd454400190188c6646cf94441f198bbc24

Comment 48

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8984203 [details]
Bug 1389251 - Do not escape spaces in nsSimpleURI

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250004/diff/2-3/

Comment 49

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8984204 [details]
Bug 1389251 - Add esc_Spaces that may be used to force escaping of spaces

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250006/diff/2-3/

Comment 50

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8984920 [details]
Bug 1389251 - Fix browser_webconsole_context_menu_copy_entire_message.js so findMessage doesn't match on the wrong thing

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250716/diff/1-2/

Comment 51

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c2c45a2a22d
Do not escape spaces in nsSimpleURI r=bz
https://hg.mozilla.org/integration/autoland/rev/180bf1c0385b
Add esc_Spaces that may be used to force escaping of spaces r=bz,jdescottes
https://hg.mozilla.org/integration/autoland/rev/58d566f57fb0
Fix browser_webconsole_context_menu_copy_entire_message.js so findMessage doesn't match on the wrong thing r=jdescottes

Comment 52

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/0c2c45a2a22d
https://hg.mozilla.org/mozilla-central/rev/180bf1c0385b
https://hg.mozilla.org/mozilla-central/rev/58d566f57fb0

Comment 53

[Levente Sacal]

Could this be tested from a QA perspective using any testcase? If not could it be marked as verified and fixed? Thanks.

Comment 54

[Valentin Gosu [:valentin] (he/him)]

Well, comment 0 it a very easy way of verifying it :) It didn't use to work before.