286028 - 'View Background Image' option available with empty CSS background-image property

Status: NEW

(Firefox :: Menus, defect, P5)

Description

[Ronnie Miller]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050225 Firefox/1.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050225 Firefox/1.0.1

If you set an element's background-image property in a stylesheet to nothing..

background-image: url();

The right-click context menu will allow you to view that element's background
image. In some cases clicking this option will open the remote stylesheet. Other
times it will open the top level of that domain. I believe the document that the
'View background image' option links to is then loaded again regardless of
whether it is an actual image.

This causes other larger problems. For example, in cases which the page is
loaded again it causes PHP and other dynamic scripting to be called upon
multiple times. If one was to have a counter or other script that increments on
each page load, the number would be incremented twice per page load.

Reproducible: Always

Steps to Reproduce:
1. Create an HTML document with a stylesheet and a div.
2. Set up an ID for the div
3. In the style definitions for the div, include the background-image property,
but do not include a URL for the image. (background-image: url();)
4. Give the div a width, height, and a border. This is just so you will be able
to see it.
5. Right click anywhere on the div and note that the 'View background image'
option is available.
Actual Results:  
Clicking on the view background image option will reload the document.
In some cases clicking it would load an external stylesheet, I have not been
able to reproduce this, but I think it may have something to do with iframes.


Expected Results:  
The software should not have allowed you to click the 'View background image' as
there is no image to view. Also, the software should not load a document for the
view background option, this causes un-desirable results with scripting and
other dynamic files as described above.

Comment 1

[Gabriel Sjoberg]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050831
Firefox/1.0+ ID:2005083106

This behavior persists in the above nightly.  I consistently get the external
stylesheet for the current page.

Comment 2

[Martijn Wargers (dead)]

It's doing a computed style check to see if the background-image is there:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#448
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#550

Background-image: url() gives a computed style of the url where the page is seen.
So the code needs a check that the computed background-image style is also not the same as the url of the page in question or something like that.

Comment 3

[Martijn Wargers (dead)]

Attachment: testcase

Comment 4

[Emil Ivanov]

(In reply to comment #2)
> It's doing a computed style check to see if the background-image is there:
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#448
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#550
> 
> Background-image: url() gives a computed style of the url where the page is
> seen.
> So the code needs a check that the computed background-image style is also not
> the same as the url of the page in question or something like that.

Better to check if the url value is not empty and valid

Comment 5

[Emil Ivanov]

This looks fixed by another bug in latest trunk:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090829 Minefield/3.7a1pre

Comment 6

[Emil Ivanov]

Also in latest branch:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a2pre) Gecko/20090828 Namoroka/3.6a2pre

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

We don't know any patch at the moment which has been fixed this problem. This makes this bug WFM.

Comment 8

[Emil Ivanov]

It seems there is a bug 482941 which causes the view background image menuitem to be always disabled, I've only checked the testcase which was with disabled option. So this is definitely not fixed.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks for checking it again. You are correct. Please leave the version info at 1.0 because it was the version when this bug was first reported.

Comment 10

[Emil Ivanov]

Done

Comment 11

[Sam Foster [:sfoster] (he/him)]

:Dao, bug 544710 seems controversial and still open for debate. In the meantime, this bug is a papercut which it seems we could address in ContextMenuChild.jsm with little to no fallout. Do you mind if I unhook it from 544710 and mentor this one?

Comment 12

[Dão Gottwald [:dao]]

I don't think we shuld spend time on this. It's not even clear to me how we'd address this. Comment 2 doesn't seem correct at a first glance, because afaik the same URL could deliver different content based on the accept header.

Comment 13

[Sam Foster [:sfoster] (he/him)]

(In reply to Dão Gottwald [::dao] from comment #12)

I don't think we shuld spend time on this. It's not even clear to me how we'd address this. Comment 2 doesn't seem correct at a first glance, because afaik the same URL could deliver different content based on the accept header.

I take your point. Unless there's a practical way to see the original, un-resolved url() value and know that it was empty, the intention becomes a bit ambiguous here.