301329 - Image-related items in context menu do not appear when images are disabled or for broken images.

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Matt Nordhoff (aka Peng on IRC & forums)]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4) Gecko/20050719 Firefox/1.0+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4) Gecko/20050719 Firefox/1.0+

If I go to a page with a broken image on it and right-click on the image, the
image-related context menu items (View Image, Copy Image Location, etc.) don't
appear, as they should. They do appear perfectly fine for images that are
working, though.


Also, just to say, I've never submitted a bug before, so please don't hurt me if
I did it all wrong or something.

Reproducible: Always

Steps to Reproduce:
1. Go to a page with a broken image.
2. Right-click on it.
Actual Results:  
The context menu appeared, but it did not have the image-related items.

Expected Results:  
The context menu should have appeared with the image-related items.

Tested with default theme and no extensions, in recent (within the last week,
maybe) Linux trunk nightlies.

Comment 1

[Matt Nordhoff (aka Peng on IRC & forums)]

(In reply to comment #0)
> User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4)
Gecko/20050719 Firefox/1.0+
> Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4)
Gecko/20050719 Firefox/1.0+
> 
> If I go to a page with a broken image on it and right-click on the image, the
> image-related context menu items (View Image, Copy Image Location, etc.) don't
> appear, as they should. They do appear perfectly fine for images that are
> working, though.
> 
> 
> Also, just to say, I've never submitted a bug before, so please don't hurt me if
> I did it all wrong or something.
> 
> Reproducible: Always
> 
> Steps to Reproduce:
> 1. Go to a page with a broken image.
> 2. Right-click on it.
> Actual Results:  
> The context menu appeared, but it did not have the image-related items.
> 
> Expected Results:  
> The context menu should have appeared with the image-related items.
> 
> Tested with default theme and no extensions, in recent (within the last week,
> maybe) Linux trunk nightlies.

Oh, actually, it looks like there was one extension in my blank profile, but
still, I doubt it or the extensions I have in my main profile are related to the
issue.

Also, if the dead image is also a link, you can click on Properties in the
context menu and it will show the stuff related to the image there...

Comment 2

[PikeUK]

Confirming, this also applies when browsing with images disabled, it is no
longer possible to view a particular image by right-clicking it and selecting
View Image.

Regression Window:
Works: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b3) Gecko/20050713 Firefox/1.0+
Fails: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b3) Gecko/20050714 Firefox/1.0+

The items appear again after reverting the changes from Top Secret Bug 293527.

Comment 3

[PikeUK]

Since I can't comment in Bug 293527 I'm CCing the people from its checkin
comment, I hope that's OK, apologies if it's not.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

maybe that should've only disabled save, not all image-related menu items...

Comment 5

[Boris Zbarsky [:bzbarsky]]

So... This raises the question of what options should be available on what
images.  Should "view image" be available on a broken image?  I don't think so.
 But it should be available on a blocked image...

What we need here is a UI spec (neil?  mconnor?).  Then
nsIImageLoadingContent.imageBlockingStatus and status of the primary request can
be used to implement said UI spec.

Not a core bug, so not a 1.8b4 issue.  But Seamonkey may need similar changes,
of course.

Comment 6

[Asa Dotzler [:asa]]

is this the same as bug 299881

Comment 7

[Boris Zbarsky [:bzbarsky]]

No.

Comment 8

[Matt Nordhoff (aka Peng on IRC & forums)]

So, the reason the stuff isn't available is that weird stuff in the <img src="">
can pose a security risk, and bug 293527 is about that? Huh. Well, it looks like
Properties is still available (I could swear I didn't see it before), which
would mean I could still view a broken image, though it would be a bit less
convenient. WONTFIX, maybe?

Comment 9

[Boris Zbarsky [:bzbarsky]]

This shouldn't be wontfix.  I should be able to "view image" on a blocked image
to view it, for example.

Comment 10

[Matt Nordhoff (aka Peng on IRC & forums)]

Right. Forgot that the bug had been expanded to cover blocked images, too. Yes,
you should be able to view them. It doesn't even look like the item to unblock
images from that server is available. That's not right. Also, why isn't there
some sort of icon for blocked images?

Comment 11

[Matt Nordhoff (aka Peng on IRC & forums)]

"view them" as in "right-click on them and hit View Image", of course. :-)

Comment 12

[[e-mail]]

Attachment: Patch v0.1a

Comment 13

[Boris Zbarsky [:bzbarsky]]

So... Should an <input type="text"> (which is instanceof nsIImageLoadingContent)
really give image context menu options?  I think you want instanceof
nsIImageLoadingContent and a non-null uri...

Comment 14

[Mike Connor [:mconnor]]

Comment on attachment 190638 [details] [diff] [review]
Patch v0.1a

>@@ -3973,7 +3974,7 @@ nsContextMenu.prototype = {
>         if (shell)
>           haveSetDesktopBackground = true;
> #endif
>-        this.showItem( "context-setDesktopBackground", haveSetDesktopBackground && this.onImage );
>+        this.showItem( "context-setDesktopBackground", haveSetDesktopBackground && this.onLoadedImage );
> 
>         if ( haveSetDesktopBackground && this.onImage ) {

this should change to onLoadedImage too, otherwise looks good.	Thanks!

Comment 15

[[e-mail]]

Attachment: Patch v0.1b

Addressed mconnor, biesi and bz's comments.

Comment 16

[[e-mail]]

Comment on attachment 190707 [details] [diff] [review]
Patch v0.1b

regression fix

Comment 17

[[e-mail]]

Fix checked in.

Comment 18

[Matt Nordhoff (aka Peng on IRC & forums)]

Yup, looks like it works fine. Thank you. :)

And just to say, this was fun. :) Everybody was nice, and the bug got taken care
of in ten days. Sounds good to me.

Comment 19

[PikeUK]

*** Bug 303242 has been marked as a duplicate of this bug. ***

Comment 20

[Daniel Veditz [:dveditz]]

aviary101/moz17 landing approval: a=dveditz for drivers. Please add the fixed1.7.13 and fixed-aviary1.0.8 keywords when landed.

Comment 21

[Daniel Veditz [:dveditz]]

Changed to relatively bogus component to get the blocking flags

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Apparently this caused bug 333035 on the old branches.

Comment 23

[Jay Patel [:jay]]

Adding verified-aviary1.0.8 keyword based on testing of latest 1.0.8 respins (based on testcases from verified bug 333035, bug 333131, and bug 333305).