[e10s] "View Image/Video" in remote browser causes unsafe CPOW usage warning

RESOLVED FIXED in Firefox 39

Status

()

Firefox
Menus
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Kwan, Assigned: Kwan)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 39
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10sm8+, firefox39 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

3 years ago
+++ 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/>
(Assignee)

Comment 1

3 years ago
Created attachment 8568054 [details] [diff] [review]
Make "View Image/Media" command use messages to avoid unsafe CPOW warnings

Relies on the gContextMenuContentData.documentURI addition in bug 1133577.
Assignee: nobody → moz-ian
Status: NEW → ASSIGNED
Attachment #8568054 - Flags: review?(mconley)
(Assignee)

Comment 2

3 years ago
Created attachment 8571677 [details] [diff] [review]
Make "View Image/Media" command use messages to avoid unsafe CPOW warnings

Updated for latest bug 1133577 patch
Attachment #8568054 - Attachment is obsolete: true
Attachment #8568054 - Flags: review?(mconley)
(Assignee)

Updated

3 years ago
Depends on: 1133577
(Assignee)

Comment 3

3 years ago
Created attachment 8573592 [details] [diff] [review]
Make "View Image/Media" command use messages to avoid unsafe CPOW warnings

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 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

3 years ago
Created attachment 8574192 [details] [diff] [review]
Make "View Image/Video" command use messages to avoid unsafe CPOW warnings

(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)
(Assignee)

Updated

3 years ago
Blocks: 1134424
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+
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
Last Resolved: 3 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.