293758 - Context menu screw-up with data and javascript protocol images

Status: VERIFIED FIXED

(Firefox :: Menus, defect, P2)

Description

[Martijn Wargers (dead)]

Right-clicking on the xbm image (the image with the letters XBM, it is an image
with a javascript: protocol) doesn't give the right context menu anymore on the
url testcase.

This doesn't happen in 2005-01-28 build, but it happens in 2005-03-01 build:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-02-28+07%3A00%3A00&maxdate=2005-03-01+09%3A00%3A00&cvsroot=%2Fcvsroot
My guess: this is a fall-out from bug 278772.

Comment 1

[Martijn Wargers (dead)]

Attachment: Testcase

This is a testcase with a data protocol image embedded.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Ok, the problem is that images from javascript: or data: don't have a "host", so
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/base/content/browser.js&rev=1.422&mark=3927#3907
fails. What is expected behavior? Hide the context menu item for such images?
Previously, the context menu showed "Block images from image" (data:) or "Block
images from <host>" (javascript:) and had no effect.

Comment 3

[Daniel C]

Context Menu -> View Image gives me 2 errors in the console:

Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIURI.host]"  nsresult: "0x80004005
(NS_ERROR_FAILURE)"  location: "JS frame :: chrome://browser/content/browser.js
:: anonymous :: line 3713"  data: no]

Error: gContextMenu has no properties
Source File: chrome://browser/content/browser.xul
Line: 1

Middle-clicking it instead of left-clicking gives me:

Error: gContextMenu has no properties
Source File: chrome://browser/content/utilityOverlay.js
Line: 223

instead of the second one above.

Following that link shows me:

// Used as an onclick handler for UI elements with link-like behavior.
// e.g. onclick="checkForMiddleClick(this, event);"
function checkForMiddleClick(node, event)
{
  if (event.button == 1) {
    /* Execute the node's oncommand.
     *
     * Using eval() because of bug 246720.  Would like to use node.oncommand(event).
     *
     * Since we're using eval():
     *
     * |event| is correct because the name of this function's formal parameter
matches
     * the automatic name of the formal parameter for oncommand, |event|.
     *
     * |this| is incorrect.  To make it correct, we would have to use Function.call.
     */
    eval(node.getAttribute("oncommand"));

    // If the middle-click was on part of a menu, close the menu.
    // (Menus close automatically with left-click but not with middle-click.)
    closeMenus(event.target);
  }
}

But that's another bug.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Attachment: Patch

Comment 5

[Mike Connor [:mconnor]]

Comment on attachment 183690 [details] [diff] [review]
Patch

r=me with the comment changed to:

// this throws if the image URI doesn't have a host (eg, data: image URIs)
// see bug 296758 for details

"//xxx" is more of a fixme comment, where this is the right fix.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Attachment: Patch with comment change

New patch addressing review comment, carrying over mconnor's r+ and asking for
1.1a2 approval for a regression fix.

Comment 7

[Daniel Brooks [:db48x]]

fix checked in

Comment 8

[Jeff Walden [:Waldo]]

(In reply to comment #5)
> (From update of attachment 183690 [details] [diff] [review] [edit])
> // see bug 296758 for details

The bug number here was mistyped, and the typo survived into the actual checkin.
 The current tree state may make fixing this a bit annoying, but it *is* a
risk-free fix.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Thanks for noticing that Jeff, I've checked in the correction.

Comment 10

[Greg K.]

Verified fixed using Win FF 1.5.