88918 - Page Source not available in context menu for linked image

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P4)

Description

[sairuh (rarely reading bugmail)]

unsure if this is expected behavior. doesn't seem to make sense if it is...

1. go to a framed page which contains an linked image, such http://faqs.org
2. in the left-hand navigation frame, bring up the context menu over the "NO
FRAMES" linked image.
3. compare with a context menu brought up over a link in a frame.

results:

for the linked image, the context menu has

* View Frame Source
* View Page Info
* View Frame Info
* View Image [expected for context menus over images]

shouldn't "View Page Source" appear above "View Frame Source", as in the context
menu for links?

Comment 1

[Boris Zbarsky [:bzbarsky]]

See
http://lxr.mozilla.org/seamonkey/source/xpfe/communicator/resources/content/nsContextMenu.js#132

That's due to the fix to bug 25497 -- a misguided fix in my opinion.  What I
would suggest doing is backing that fix out, reopening bug 25497, and making it
depend on bug 73847.  That way we can _really_ tell when we're viewing an
image-only document.

ccing mpt and gerv so they can comment.

Comment 2

[sairuh (rarely reading bugmail)]

iirc, i had tested bug 25497 when the image was the only object in the page, eg,
http://faqs.org/images/noframes.gif

this bug was found when i was viewing http://faqs.org

Comment 3

[Boris Zbarsky [:bzbarsky]]

The "fix" for bug 25497 just removed the "view source" context menu option if
the right-click is on an image.  You can see this with the image banner on
http://www.mozilla.org, even.  At the same time, right-clicking off the image in
image-only documents leaves that option in the menu.  Hence the suggested
dependency on bug 73847...

Comment 4

[Gervase Markham [:gerv]]

I wholeheartedly agree with bzbarsky - his suggested course of action is 
probably the correct one. My fix sucked. 

Alternatively, we could do what Page Info does and use nsIMIMEService, which 
would be better than nothing.

In either case, we should try and get a bit more pressure put on to fix bug 
73847.

Gerv

Comment 5

[Boris Zbarsky [:bzbarsky]]

OK.  I have a fix for bug 73847.  Once I check it in, I can fix this.  Taking...

blake, is it OK if I mess with context menu code a bit?  Or do you have major 
changes in your tree implementing mpt's spec and would go through merge hell if 
it got changed?

Comment 6

[Matthew T (active 1999-2002)]

This bug is invalid. The context menu for a linked image should only contain
items concerning the link and items concerning the image, and `Page Source' is
neither. Currently it contains all sorts of crap which shouldn't be there --
it's 22 items long, which is about twice as long as the maximum acceptable for a
context menu.

Comment 7

[Boris Zbarsky [:bzbarsky]]

mpt, how is your context menu spec doing?  Should we go ahead and start 
implementing parts of it?

Comment 8

[Asa Dotzler [:asa]]

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)

Comment 9

[Matthew T (active 1999-2002)]

Boris, yes please. This menu has been far too long for far too long. :-)
<http://mozilla.org/projects/ui/menus/shortcut/>

Comment 10

[sairuh (rarely reading bugmail)]

boris, also check out
http://www.mozilla.org/projects/ui/communicator/framework/contextmenus/cmrev2.html

and bug 75338, if you're brave. ;)

Comment 11

[Lori Kaplan]

The currently agreed upon context menu spec is the one mentioned by sairuh.
http://www.mozilla.org/projects/ui/communicator/framework/contextmenus/cmrev2.html
This updated spec is based on mpt's spec and evolves it according to principles
agreed upon in pixeljockeys. The spec is open for comment and discussion. The
plan is to work on implementation in the 0.9.9 timeframe. Please don't implement
mpt's older spec at this time. 

Comment 12

[Blake Ross]

> The currently agreed upon context menu spec is the one mentioned by sairuh

Currently agreed upon by who? It was just posted publicly two days ago.  mpt's 
spec has been around since July and has been revised six times in response to 
multiple newsgroup discussions, as well as a flood of comments in bug 75338.  
It seems unfair to mpt to say that a spec that few have had a chance to even 
read over yet is the "currently agreed upon" one.  Unless, of course, you mean 
agreed upon by both marlon and mpt, but that seems unlikely given mpt's linking 
to his spec in his comment from today.

> This updated spec is based on mpt's spec and evolves it according to 
> principles agreed upon in pixeljockeys.

I thought the most significant PJs principle was to compromise, and judging 
from this game of use-my-spec-no-use-mine, this hasn't happened yet...

Comment 13

[Lori Kaplan]

Agreed upon at the pixeljockeys meetings of late. mtp did not attend the last
one, but trudelle spoke with him afterwards regarding the spec. Many attempts
were made to have marlon and mpt agree. And you're right about compromise.
That's how the most recent spec was devised. 

Comment 14

[Blake Ross]

Okay; I guess if both parties are in agreement, there's no cause for concern.  
(Odd that mpt would link to his spec as recently as today, though...)

It's irksome to see six month's worth of discussion and revision tossed out the 
door for a spec that does not yet, by its creator's own admission, take into 
account any of that discussion.  (The Jan. 1, 2002 deadline for discussion 
imposed in bug 75338 is also rather inane. I know of a number of intelligent 
individuals who will be away for the holidays.)

This entire process has been a tremendous resource-waster for all involved.  
Hopefully things will go more smoothly in the future.

Comment 15

[Boris Zbarsky [:bzbarsky]]

sairuh, I have no plans of doing anything with this bug until bug 75338 (which I 
_am_ following) gets somewhere.  There is just no point.

Blake, do you just want to take this, as long as you'll be redoing all the 
context menu stuff?

Comment 16

[Peter Trudelle]

We're in no hurry to rush Marlon's spec through, the intent is for it to be the
natural heir to both the original approved spec, and MPT's alternate. 
Hopefully, they are collaborating so that it will contain viable solutions to
the most important of the issues that have been raised. 

Comment 17

[Blake Ross]

-> me

Comment 18

[Blake Ross]

Fixed (in that there's now consistency).

Comment 19

[sairuh (rarely reading bugmail)]

yep, page source is no longer in the image link context menu. verifying.