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

RESOLVED FIXED

Status

()

Core
Selection
--
minor
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: Tarmo Järvalt, Assigned: johann.petrak@gmail.com)

Tracking

Trunk
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

15 years ago
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

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

Comment 2

15 years ago
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.
(Assignee)

Updated

15 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
(Assignee)

Comment 3

15 years ago
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.
(Assignee)

Comment 4

15 years ago
Created attachment 108498 [details] [diff] [review]
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 ... :)
(Assignee)

Updated

15 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 5

15 years ago
oops, messed up - taking for now, lets see what the reviewers think of this.
Assignee: mjudge → johann
Status: ASSIGNED → NEW
(Assignee)

Updated

15 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

15 years ago
Attachment #108498 - Flags: review?(sgehani)
*** Bug 187415 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 7

15 years ago
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.
Summary: showItem("context-copy") should check isTextSelected → showItem("context-copy") should check isTextSelected and onTextInput
(Assignee)

Updated

15 years ago
Attachment #108498 - Attachment is obsolete: true
Attachment #108498 - Flags: review?(sgehani)
(Assignee)

Comment 8

15 years ago
Created attachment 114867 [details] [diff] [review]
corrected patch
(Assignee)

Updated

15 years ago
Attachment #114867 - Flags: review?(blaker)
Attachment #114867 - Flags: superreview?(jaggernaut)
Attachment #114867 - Flags: review?(timeless)
Attachment #114867 - Flags: review?(blaker)

Updated

15 years ago
Attachment #114867 - Flags: review?(timeless) → review+

Updated

15 years ago
Attachment #114867 - Flags: superreview?(jaggernaut) → superreview+

Comment 9

15 years ago
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').
(Assignee)

Updated

15 years ago
Whiteboard: [needs checkin]
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
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
Whiteboard: [needs checkin]

Comment 11

15 years ago
... (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?
You need to log in before you can comment on or make changes to this bug.