The default bug view has changed. See this FAQ.

'View Background Image' context-menu item is always greyed out

RESOLVED FIXED in Firefox 3.7a1

Status

()

Firefox
Menus
P2
normal
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: philippe (part-time), Assigned: dao)

Tracking

({regression})

Trunk
Firefox 3.7a1
regression
Points:
---
Bug Flags:
blocking-firefox3.6 +

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
STR:
1. load http://www.mozilla.com/en-US/
2. right click somewhere on the big background image

ER: context menu item 'View Background Image' is active
AR: 'View Background Image' is greyed out.

Additionally: the background images don't show up in the media tab of the page info window

Works
http://hg.mozilla.org/mozilla-central/rev/d9d4bf676f65
Gecko/20090219 Minefield/3.2a1pre

Fails
http://hg.mozilla.org/mozilla-central/rev/35d78a340566
Gecko/20090220 Minefield/3.2a1pre

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d9d4bf676f65&tochange=35d78a340566

Possibly bug 322475 ? (in which case, this probably should be a core bug)

Updated

8 years ago
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 422851
This should not be marked as a duplicate of a bug predating the regression.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
This is probably because the getComputedURL function doesn't handle background-image's computed value being a list, as it now is.  (And I'd strongly prefer avoiding CSSPrimitiveValue use anyway.)
Blocks: 322475
Flags: blocking-firefox3.6?
Component: General → Menus
QA Contact: general → menus
(In reply to comment #3)
> (And I'd strongly prefer avoiding CSSPrimitiveValue use anyway.)

On further consideration, I think it's probably better than the alternatives.

Comment 5

8 years ago
> This is probably because the getComputedURL function doesn't handle
> background-image's computed value being a list, as it now is.

It's failing here as well |this.target instanceof HTMLHtmlElement|
Status: REOPENED → NEW
(In reply to comment #5)
> > This is probably because the getComputedURL function doesn't handle
> > background-image's computed value being a list, as it now is.
> 
> It's failing here as well |this.target instanceof HTMLHtmlElement|

If you're seeing it failing there, then either you're debugging it wrong or you're seeing a different problem.

I can tell you pretty much for sure that the multiple backgrounds patch would have broken this for the reason in comment 3.
(Assignee)

Updated

8 years ago
Assignee: nobody → dao
Summary: [regression] Page with background-image: context-menu item 'View Background Image' greyed out → 'View Background Image' context-menu item is always greyed out
(Assignee)

Comment 7

8 years ago
Created attachment 379606 [details] [diff] [review]
patch

This just takes the first background image, which is bad when sites start to use multiple background images extensively, but should do for now.
Attachment #379606 - Flags: review?(mano)
Attachment #379606 - Flags: review?(mano) → review-
Comment on attachment 379606 [details] [diff] [review]
patch

This is actually pretty bad. I cannot think of any good reason to pick the first background and not the second.

It's probably the right time to move this feature to the page info dialog. Ask ux.
(Assignee)

Updated

8 years ago
Assignee: dao → nobody
Keywords: uiwanted
Until the right UI is worked out, a menu item that shows the right image in 99% of all cases is preferable to a menu item that's always greyed out, I think.
(Reporter)

Updated

8 years ago
Duplicate of this bug: 509658
(In reply to comment #9)
> Until the right UI is worked out, a menu item that shows the right image in 99%
> of all cases is preferable to a menu item that's always greyed out, I think.

I agree; multiple backgrounds are still going to rare on the Web for quite a while, since IE doesn't support them.

Comment 12

8 years ago
Actually, in my report of this bug, marked as a dupe, I was reporting this as an issue of a single background.  The image is a png from the body section of a linked-in stylesheet. The bug I submitted was https://bugzilla.mozilla.org/show_bug.cgi?id=509658
Blocks: 286028
No longer blocks: 286028

Comment 13

8 years ago
Another comment from me :D I'm running the latest build of Minefield/Namoroka whathaveyou - 3.6a2pre.  If you browse to http://newsofthebrutal.org/ and right-click, you should see the grayed out option for "View Background Image."
We should either fix this, or remove the menuItem. Having it always greyed out is helping no one.

David: does this belong here or in another component?
Flags: blocking-firefox3.6? → blocking-firefox3.6+
We have context menu tests, but I guess this particular one wasn't handled. Would be good to add a test for View Background Image being both present and enabled.
Flags: blocking-firefox3.6+ → blocking-firefox3.6?
Seems to me like it's in the right component.
Oh, and it looks like dolske reset beltzner's setting the blocking flag to +; let's see if I have the permissions to fix that...
Flags: blocking-firefox3.6? → blocking-firefox3.6+
UX suggestions:

- when there's only one background image, we should just show the item as usual
- when there are multiple background images, we should not show the menuItem
- as a followup, we should have a new menuItem "View Background Images" which open the multiple images in tabs
Assignee: nobody → dao

Comment 19

8 years ago
(In reply to comment #18)
> - when there are multiple background images, we should not show the menuItem
> - as a followup, we should have a new menuItem "View Background Images" which
> open the multiple images in tabs

Instead, how about "View Background Image" on the background image over which right-clicking occurred?
(Assignee)

Comment 20

8 years ago
Created attachment 399569 [details] [diff] [review]
patch
Attachment #379606 - Attachment is obsolete: true
Attachment #399569 - Flags: review?(mano)
(Assignee)

Updated

8 years ago
Keywords: uiwanted

Comment 21

8 years ago
(In reply to comment #19)
That might work, but what about overlapping backgrounds with transparency? I think opening multiple in tabs would be a bit better, or perhaps a submenu:
View Background Images... -> bg1.jpg, bg2.jpg, ... and leave the tabbing up to the user - middle-click or ctrl+click an image from the list to open in a new tab or on the whole "View Background Images..." to open all in tabs.
Everyone please remember we're talking about something that isn't widely supported on the web, so we're into corner cases and assumptions about how things will work. The more important aspect of this bug, IMO, is restoring the function for the 90+% of cases out there where there's a single background image. Follow-ups can be filed on changing the behaviour of the multiple background case if that comes up a lot in the future.
Comment on attachment 399569 [details] [diff] [review]
patch

For Now.


r=mano.
Attachment #399569 - Flags: review?(mano) → review+
(Assignee)

Comment 24

8 years ago
http://hg.mozilla.org/mozilla-central/rev/7bfe69ba4bc9
Status: NEW → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
Tested trunk on www.mozillazine.org with multiple backgrounds in the header; Top level images are selected before the lower background image and view background image stays greyed out when not right clicking on any image.  

So it works as expected for UI's suggested fix in comment 19.

Nice work guys.
It also works for case in comment 18 for a single background image on the page and case of comment 21 with two background images where the top level background image looks transparent on www.mozillazine.org.

Though I found a case it doesn't do anything and maybe just an edge case bug in itself.  Where if you right click on a bug page such as this, it has 2 image labeled as background, but I cannot right click on Mozilla image above 
and see the generic image context menu items, nor do I see the view-background image context menu item, even though it appears to be on top of the main page background image and labeled as a background type in page info.  Maybe its actually below it?
Priority: -- → P2
(Assignee)

Comment 27

8 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/303d5d3380b4
status1.9.2: --- → beta1-fixed
You need to log in before you can comment on or make changes to this bug.