JSON Viewer will still load in a child frame of a content page

RESOLVED FIXED in Firefox 58

Status

()

Firefox
Developer Tools: JSON Viewer
P3
normal
RESOLVED FIXED
10 months ago
2 months ago

People

(Reporter: Cody Crews, Assigned: Oriol)

Tracking

56 Branch
Firefox 58
Points:
---

Firefox Tracking Flags

(firefox57 fix-optional, firefox58 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

10 months ago
Created attachment 8890353 [details]
jsonViewer-iframe-testcase-new.html

Using the mime type of application/vnd.mozilla.json.view it's still possible to have the json viewer load as a child of a content webpage.

See attached testcase for this.

I thought this was handled some time back, or maybe we don't need this protection anymore?
(Assignee)

Comment 1

10 months ago
Simpler testcase:
data:text/html,<iframe src="data:application/vnd.mozilla.json.view,1">

I thought this was intended, when I refactored the saving functionality in bug 1327784, I took this case into account. It complicated the code a little bit, because the window ID was necessary in order to know which window to save. Other functionalities seem to work properly too, but adding some tests would be a good idea.

Or maybe just disable it, yes. Might be more consistent because application/json does not load the JSON Viewer inside an iframe.
status-firefox57: --- → fix-optional
Priority: -- → P3
Comment hidden (mozreview-request)
(Assignee)

Comment 3

8 months ago
If the type is application/vnd.mozilla.json.view in an iframe, the patch changes it to application/json so that the JSON Viewer does not load. I also removed the code that allowed saving the JSON Viewer in iframes.
Assignee: nobody → oriol-bugzilla
Status: NEW → ASSIGNED

Comment 4

8 months ago
mozreview-review
Comment on attachment 8910210 [details]
Bug 1384572 - Prevent JSON Viewer from loading in iframes.

https://reviewboard.mozilla.org/r/181698/#review187462

Looks reasonable to me, thanks for fixing it.

R+ assuming try is green.

Honza

::: devtools/shim/devtools-startup.js:634
(Diff revision 1)
>      } else {
>        // The following code emulates saveBrowser, but:
>        // - Uses the given blob URL containing the custom contents to save.
>        // - Obtains the file name from the URL of the document, not the blob.
>        let persistable = browser.frameLoader;
> -      persistable.startPersistence(message.data.windowID, {
> +      persistable.startPersistence(0, {

Is it ok to pass 0 here?
Attachment #8910210 - Flags: review?(odvarko) → review+
(Assignee)

Comment 5

8 months ago
(In reply to Jan Honza Odvarko [:Honza] from comment #4)
> Is it ok to pass 0 here?

Yes, from http://searchfox.org/mozilla-central/source/dom/webbrowserpersist/nsIWebBrowserPersistable.idl#28-29,

> If set at 0, nsIWebBrowserPersistable will attempt to persist
> the top-level document.
Keywords: checkin-needed

Comment 6

8 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7eaea95410f2
Prevent JSON Viewer from loading in iframes. r=Honza
Keywords: checkin-needed

Comment 7

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7eaea95410f2
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58

Comment 8

5 months ago
I have reproduced this bug with Nightly 56.0a1 (2017-07-26) on Windows 10, 64 Bit!

This bug's fix is verified with latest Beta!

Build ID   : 20171218174357
User Agent : Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
QA Whiteboard: [bugday-20171220]

Comment 9

5 months ago
I have reproduced this bug with Nightly 56.0a1 (2017-07-26) on ubuntu 16.04

The bug's fix is now verified with Latest Beta 58.0b12

Build ID  20171218174357
User Agent Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0

[bugday-20171220]
Duplicate of this bug: 1447654
You need to log in before you can comment on or make changes to this bug.