Closed Bug 293758 Opened 17 years ago Closed 17 years ago

Context menu screw-up with data and javascript protocol images


(Firefox :: Menus, defect, P2)






(Reporter: martijn.martijn, Assigned: Gavin)




(Keywords: regression, testcase)


(2 files, 1 obsolete file)

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:
My guess: this is a fall-out from bug 278772.
Attached file Testcase
This is a testcase with a data protocol image embedded.
No longer blocks: 278772
Depends on: 278772
Ok, the problem is that images from javascript: or data: don't have a "host", so
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.
Assignee: nobody →
Priority: -- → P2
Target Milestone: --- → Firefox1.1
Context Menu -> View Image gives me 2 errors in the console:

Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) []"  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
     * the automatic name of the formal parameter for oncommand, |event|.
     * |this| is incorrect.  To make it correct, we would have to use

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

But that's another bug.
Component: General → Menus
OS: Windows 2000 → All
Hardware: PC → All
Attached patch Patch (obsolete) — Splinter Review
Attachment #183690 - Flags: review?(mconnor)
Whiteboard: [patch-r?]
Flags: blocking1.8b3?
Comment on attachment 183690 [details] [diff] [review]

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.
Attachment #183690 - Flags: review?(mconnor) → review+
New patch addressing review comment, carrying over mconnor's r+ and asking for
1.1a2 approval for a regression fix.
Attachment #183690 - Attachment is obsolete: true
Attachment #186154 - Flags: review+
Attachment #186154 - Flags: approval-aviary1.1a2?
Attachment #186154 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Whiteboard: [patch-r?] → [patch-r+][checkin needed][a+]
fix checked in
Closed: 17 years ago
Resolution: --- → FIXED
Flags: blocking1.8b3?
Whiteboard: [patch-r+][checkin needed][a+]
(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.
Thanks for noticing that Jeff, I've checked in the correction.
Verified fixed using Win FF 1.5.
QA Contact: general → menus
You need to log in before you can comment on or make changes to this bug.