Closed
Bug 1134391
Opened 9 years ago
Closed 9 years ago
[e10s] "View Image/Video" in remote browser causes unsafe CPOW usage warning
Categories
(Firefox :: Menus, defect)
Firefox
Menus
Tracking
()
RESOLVED
FIXED
Firefox 39
People
(Reporter: Kwan, Assigned: Kwan)
References
Details
Attachments
(1 file, 3 obsolete files)
3.07 KB,
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1133577 +++ STR: 1) Visit a site with images, <canvas/>, or <video/>s on it in an e10s window 2) Right-click on an image, <canvas/> or <video/>, and choose "View Image/Video" This causes some "unsafe CPOW usage" warnings in the Browser Console. In browser/base/content/nsContextMenu.js: // Change current window to the URL of the image, video, or audio. viewMedia: function(e) { var viewURL; if (this.onCanvas) viewURL = this.target.toDataURL(); <-- Causes CPOW warning [canvas] else { viewURL = this.mediaURL; urlSecurityCheck(viewURL, this.browser.contentPrincipal, Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT); } var doc = this.target.ownerDocument; <-- Causes CPOW warning openUILink(viewURL, e, { disallowInheritPrincipal: true, referrerURI: doc.documentURIObject }); <-- Causes CPOW warning }, in toolkit/modules/RemoteWebNavigation.jsm loadURI: function(aURI, aLoadFlags, aReferrer, aPostData, aHeaders) { if (aPostData || aHeaders) throw Components.Exception("RemoteWebNavigation doesn't accept postdata or headers.", Cr.NS_ERROR_INVALID_ARGS); this._browser._contentTitle = ""; this._sendMessage("WebNavigation:LoadURI", { uri: aURI, flags: aLoadFlags, referrer: aReferrer ? aReferrer.spec : null <-- Causes CPOW warning }); }, warning tagged [canvas] only occurs on a <canvas/>
Updated•9 years ago
|
Assignee | ||
Comment 1•9 years ago
|
||
Relies on the gContextMenuContentData.documentURI addition in bug 1133577.
Assignee | ||
Comment 2•9 years ago
|
||
Updated for latest bug 1133577 patch
Attachment #8568054 -
Attachment is obsolete: true
Attachment #8568054 -
Flags: review?(mconley)
Assignee | ||
Comment 3•9 years ago
|
||
Well this is the next one in my queue, so this can go first. toDataURL is a separate function so it can be reused for "Save Image As..."ing a canvas later. Is Cu.reportError the right thing to put for the promise rejection?
Attachment #8571677 -
Attachment is obsolete: true
Attachment #8573592 -
Flags: review?(mconley)
Comment 4•9 years ago
|
||
Comment on attachment 8573592 [details] [diff] [review] Make "View Image/Media" command use messages to avoid unsafe CPOW warnings Review of attachment 8573592 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/nsContextMenu.js @@ +1026,5 @@ > }, > > + toDataURL: function(aTarget) { > + let mm = this.browser.messageManager; > + return new Promise(function(resolve, reject) { You never use reject, so you can probably drop it. @@ +1027,5 @@ > > + toDataURL: function(aTarget) { > + let mm = this.browser.messageManager; > + return new Promise(function(resolve, reject) { > + mm.sendAsyncMessage("ContextMenu:ToDataURL", {}, { aTarget }); A DataURL can represent many things. Let's call this message: ContextMenu:Canvas:ToDataURL and the function: canvasToDataURL
Attachment #8573592 -
Flags: review?(mconley) → review-
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #4) > > + return new Promise(function(resolve, reject) { > > You never use reject, so you can probably drop it. Cool, done. > @@ +1027,5 @@ > > > > + toDataURL: function(aTarget) { > > + let mm = this.browser.messageManager; > > + return new Promise(function(resolve, reject) { > > + mm.sendAsyncMessage("ContextMenu:ToDataURL", {}, { aTarget }); > > A DataURL can represent many things. Let's call this message: > > ContextMenu:Canvas:ToDataURL Done. > and the function: > > canvasToDataURL Done, plus the _ prefixing as discussed on IRC. And reverted the argument name back to "target", so the patch actually works again, tested manually.
Attachment #8573592 -
Attachment is obsolete: true
Attachment #8574192 -
Flags: review?(mconley)
Comment 6•9 years ago
|
||
Comment on attachment 8574192 [details] [diff] [review] Make "View Image/Video" command use messages to avoid unsafe CPOW warnings Review of attachment 8574192 [details] [diff] [review]: ----------------------------------------------------------------- Looks great! Thanks Ian!
Attachment #8574192 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 7•9 years ago
|
||
Try run at https://treeherder.mozilla.org/#/jobs?repo=try&revision=094396e39ebf orange caused by the other patch for bug 1141186 and fixed in https://treeherder.mozilla.org/#/jobs?repo=try&revision=d06d6f239f24
Keywords: checkin-needed
Comment 8•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/839427d11a62
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/839427d11a62
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 39
You need to log in
before you can comment on or make changes to this bug.
Description
•