182997 - showItem("context-copy") should check isTextSelected and onTextInput

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Tarmo Järvalt]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3a) Gecko/20021122
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3a) Gecko/20021122

this.showItem( "context-copy", true );
in:
/chrome/comm.jar/content/communicator/nsContextMenu.js
should be replaced by:
this.showItem( "context-copy", this.isTextSelected );

Reproducible: Always

Steps to Reproduce:
1. Load URL http://mozilla.org/
2. Right click on any link (make sure text isn't selected)
3. Context menu pops up
Actual Results:  
There is disabled (gray) item &Copy
When you want to copy selected link with C (accel for &Copy Link Location) you
have to press enter before "&Copy Link Location" is activated. &Copy is disabled
and somewhat blocking use of "&Copy Link Location".

Expected Results:  
If no text is selected then &Copy menu item shouldn't appear.

When:
this.showItem( "context-copy", true );
is replaced by:
this.showItem( "context-copy", this.isTextSelected );
then all works as expected.

This might be a regression because IIRC this disabled &Copy item didn't appear
in 1.2b or earlier nightlies.

I have no clue how to create a patch so someone should do it for me.

Comment 1

[Erin Dalzell]

Is this a regression? I am on Phoenix (recent nightly) and it WFM.

Comment 2

[George K.]

Verifying this behavior in:

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3a) Gecko/20021202

As described, right-clicking any link will bring up Copy Link Location and Copy
options, both of which use C as the activating key thus requiring an extra
keypress (ENTER) to select Copy Link Location.

I doubt this is OS specific (probably OS-All instead of WinXP) as the reporter
is using Linux and I am using Win2K.  It is definitely a minor bug, but the
proposed change seems trivial and elegant.  Removing the Copy option from this
context menu (when there is nothing hilighted) would not only fix this bug, but
would make finding the Copy Link Location option faster to find visually,
increasing usability of that option when mousing as well as when using the keyboard.

If I could confirm (Status-New) this bug, I would.  For Comment 1, I believe
that Phoenix is using its own context menu, not Mozilla's, but I'll have to look
into it to be sure.

Comment 3

[[e-mail]]

This is a special case of the fact that the context menu sometimes does not
show options and sometimes only greys them. E.g. when you click on some empty
space in a web page that does not have a background image, the context menu 
will still contain the option "view background image", but greyed. 

This probably has to do with usability considerations - some people want
to have the context menu as constant as possible, because they simply 
physically remember how to reach a certain option - if the context menu
looks different every time this is not possible. Thats something for the
more advanced users.

Other people want the context menu as concise as possible so they can visually
scan it faster - thats better for beginners.
Personally I tend to favor the second alternative, since a context menu
should, being a context menu, adapt to context :)

Somebody has to reach a decision on this.

Comment 4

[[e-mail]]

Attachment: patch that implements the proposed fix

In case this is wanted and the proposed fix is the right thing to do, 
here is the patch ... :)

Comment 5

[[e-mail]]

oops, messed up - taking for now, lets see what the reviewers think of this.

Comment 6

[Boris Zbarsky [:bzbarsky]]

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

Comment 7

[Tarmo Järvalt]

Ouch, stupid me, it should be:

-        this.showItem( "context-copy", true );
+        this.showItem( "context-copy", this.isTextSelected || this.onTextInput);

because otherwise there is no Copy menuitem in textboxes.

Comment 8

[[e-mail]]

Attachment: corrected patch

Comment 9

[Lewis Jardine]

The policy would seem to be (from observed behaviour, rather than any formal
document) that an item is grayed if it is an action that could normally be
applied to the thing that's been right-clicked, but cannot be for some reason.

Example:

Right-clicking the background produces options like 'forward', 'back', 'show
background image'. These get greyed-out if this action is impossible (There is
no page to go forward/back to, there is no background image).

Right-clicking selected text produced options like 'cut', 'copy, 'paste'. 'cut'
and 'paste' get greyed-out on most HTML documents, as they are read-only.

In this case, copying with no selected text should probably not be greyed-out,
but removed entirely, as it is an out-of-context action to perform. It's not
just imposible to copy when nothing's highlighted, but the action makes no
sense. (Except maybe to copy the text of the link, but then 'copy link text'
would be clearer).

Additionally, as it is possible (and sensible) to have 'copy link location' and
'copy' on the same menu (right-clicking a highlighted URL), I'd suggest that
'copy link location' needs a new hotkey (as it would confuse everyone if we
redefined the one for 'copy').

Comment 10

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

Checking in xpfe/communicator/resources/content/nsContextMenu.js;
/cvsroot/mozilla/xpfe/communicator/resources/content/nsContextMenu.js,v  <-- 
nsContextMenu.js
new revision: 1.81; previous revision: 1.80
done


checked in

Comment 11

[Tobias]

... (Except maybe to copy the text of the link, but then 'copy link text'
would be clearer)....

This is one thing I am often missing, just copy the link text. Could this menu
option be included in this bug as well?