1345807 - View Page Source doesn't work on data URI loaded in file content process.

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Bob Owen (:bobowen)]

I noticed this while testing bug 1343184.

This isn't necessarily just data URIs, but any URI type that is allowed to load in more that one remoteType, that you can view-source on.
At the moment it picks up the default remoteType, not the one for browser which you are viewing the source of.

I have patches.

Comment 1

[Bob Owen (:bobowen)]

Try push with the fix from bug 1343184 included (I've fixed the lint issues locally):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c4c17a736362f90ea8287fb6ed43e4b9e553d0d

Comment 2

[Bob Owen (:bobowen)]

Attachment: Part 1: When viewing source of existing browser always use its remoteType

Comment 3

[Bob Owen (:bobowen)]

Attachment: Part 2: Check that we can view-source on data URI loaded in file content process

Comment 4

[:Gijs (he/him)]

Comment on attachment 8845399 [details] [diff] [review]
Part 2: Check that we can view-source on data URI loaded in file content process

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

Please put the new test in the 'tabs' directory instead of in general/ .

::: browser/base/content/test/general/browser_viewsource_of_data_URI_in_file_process.js
@@ +13,5 @@
> +  const uriString = Services.io.newFileURI(dir).spec;
> +  let fileTab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, uriString);
> +  registerCleanupFunction(function* () {
> +    yield BrowserTestUtils.removeTab(fileTab);
> +  });

Instead of this, you could do:

yield BrowserTestUtils.withNewTab(uriString, function*(fileBrowser) {
  // rest of test in here.
});

Comment 5

[Bob Owen (:bobowen)]

Thanks for the reviews.

(In reply to :Gijs from comment #4)
> Comment on attachment 8845399 [details] [diff] [review]
> Part 2: Check that we can view-source on data URI loaded in file content
> process

> Please put the new test in the 'tabs' directory instead of in general/ .

Moved locally.

> > +  let fileTab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, uriString);
> > +  registerCleanupFunction(function* () {
> > +    yield BrowserTestUtils.removeTab(fileTab);
> > +  });
> 
> Instead of this, you could do:
> 
> yield BrowserTestUtils.withNewTab(uriString, function*(fileBrowser) {
>   // rest of test in here.
> });

Nice.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ac37612bb04
Part 1: When viewing source of existing browser always use its remoteType. r=Gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/a89a71a9bb73
Part 2: Check that we can view-source on data URI loaded in file content process. r=Gijs

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6ac37612bb04
https://hg.mozilla.org/mozilla-central/rev/a89a71a9bb73