146825 - 2 'w' accesskeys in context menu conflict - for a link while some text is selected

Status: RESOLVED WORKSFORME

(SeaMonkey :: UI Design, defect)

Description

[Olga]

Build: recent branch builds

Overview: 2 'w' conflict in context menu for links while keeping selected text. 
I nominate it for nsbeta1 since our Quick Searh on any word on the page is one
of our new features and we expect people using it.

Steps to reproduce:  
1. Select a text on Browser page. I worked with list of bugs.
2. Right click on a link. See context menu, which includes: 'Web Search for
"<selected text>"' and regular for a link: 'Open in  New Window' - 'W' mnemonic
in both menu items.
3. Press 'W' to activate one of that menu items.

Actual Results: Nothing happens.

Expected Results: Context menu item is activated by using mnemonic character.

Additional Information: I checked query for Browser, context menu - did not find
existing bug for this. (Sarah is on vacation)

Comment 1

[Matthias Versen [:Matti]]

-> Xp APPS

Comment 2

[Christopher Aillon (sabbatical, not receiving bugmail)]

Related: bug 135210.

Comment 3

[Paul Wyskoczka]

qa  --> [e-mail]

Comment 4

[Samir Gehani]

Nav triage team: nsbeta1-

Comment 5

[sairuh (rarely reading bugmail)]

could this be a dup of bug 135225?

Comment 6

[Olga]

Yes, it is related to bug 135225, but I would not duplicate it, since scenario
is different and emphasizes trouble in accessing our new feature Quick Search.

Comment 7

[sairuh (rarely reading bugmail)]

nominating for buffy...

Comment 8

[Samir Gehani]

Nav triage team: nsbeta1-

Comment 9

[Blake Ross]

Reassigning obsolete bugs to their respective Seamonkey owners (i.e. nobody). 
If you want this fixed for Firefox, change the Product and Component accordingly
and reassign back to me.

Comment 10

[Giacomo Magnini]

Attachment: Fixed Search Web for accesskey (W -> S) and fixed also another conflict for Copy Link Location (C [used also by Copy] -> k).

Comment 11

[[e-mail]]

Comment on attachment 171619 [details] [diff] [review]
Fixed Search Web for accesskey (W -> S) and fixed also another conflict for Copy Link Location (C [used also by Copy] -> k).

K is OK but S is already used by Set As Wallpaper. Maybe we need to decide not
to show quite so many items when right-clicking a selected linked image :-)
Note: A is used by both Save Page as and Select All.

Comment 12

[Giacomo Magnini]

(In reply to comment #11)
> K is OK but S is already used by Set As Wallpaper. Maybe we need to decide not
> to show quite so many items when right-clicking a selected linked image :-)
> Note: A is used by both Save Page as and Select All.

While I think that some items can reuse the same accesskey at times, we really
need some more rationale on what to give accesskeys to and what not...
Anyway, new patch coming for fixing the conflicts.

Comment 13

[Giacomo Magnini]

Attachment: Second attempt, trying to fix also the accesskey for Save Page As (A -> E).

Let's just hope that nobody will add an accesskey for Block images from this
server, or we gonna have some real troubles... :-)

Comment 14

[[e-mail]]

Canonical List Of All Context Menu Access Keys
==============================================

A	Select All

B	Back
B	Bookmark This Link
"Back" is not available on links

C	Copy

D	Delete
D	Switch Page Direction
"Switch Page Direction" is not available on text fields

E	Copy Email Address

F	Forward
F	Search Web For …
"Forward" is not available with a selection

G	Save Image

H	This Frame

I	Fit Image To Window
I	View Background Image
I	View Image
"View Background Image" is not available with a foreground image
"Fit Image To Window" is only available after "View Image"

L	Copy Link Location

M	Bookmark This Page
M	Copy Image
"Bookmark This Page" is not available with an image

O	Properties

P	Paste
P	Save Page
"Save Page" is not available on text fields

R	Redo
R	Reload
R	Set As Wallpaper
"Reload" is not available on text fields or images

S	Save Link Target
S	Stop
S	Switch Text Direction
"Stop" is not available on text fields or images

T	Cut
T	Open Link in New Tab
"Open Link in New Tab" is not available on text fields

U	Undo
U	View MathML Source
"View MathML Source" is not available on text fields

V	View Page Source
V	View Selection Source
"View Page Source" is not available with a selection

W	Open Link in New Window
W	View Page Info
"View Page Info" is not available with a link

==============================================
Any objections?

Comment 15

[Giacomo Magnini]

Attachment: Patch for Neil's suggestion

I don't seem to find the accesskey for View MathML Source...

Comment 16

[Giacomo Magnini]

Actually, bug 258437 suggets to use S for Save Image, consistent with other
browsers. And in bug 139047 it was suggested to use R for Properties, to mimic IE...

Comment 17

[[e-mail]]

(In reply to comment #16)
>Actually, bug 258437 suggets to use S for Save Image, consistent with other
>browsers. And in bug 139047 it was suggested to use R for Properties, to mimic
>IE...
S is for Stop and R is for Reload. And you probably have to create a new access
key for View MathML Source.

Comment 18

[Ian Neal]

Will Send Page... (accesskey = d) clash with Switch Page Direction?

Comment 19

[Ian Neal]

Maybe we can use 'k' as the accesskey for Block/Unblock images from this server?

Comment 20

[[e-mail]]

(In reply to comment #18)
>Will Send Page... (accesskey = d) clash with Switch Page Direction?
Yes, Switch Page Direction is only hidden on text fields. But perhaps we can use
accesskey="n" for Send Page/Image - Send Page won't appear on an image.

(In reply to comment #19)
>Maybe we can use 'k' as the accesskey for Block/Unblock images from this server?
That sounds like a good idea.

Comment 21

[timeless]

how about *not* having send page (or send image) in any context menus? if
someone wants to send a page, let them use the file menu.

Comment 22

[Giacomo Magnini]

More head scratching:
1) If we use K for block images, we can't use them in the MailNews context menu
(already used by Mark)
2) Save Frames As... (in This Frame submenu) uses m as Bookmark this frame
(switch to F)?
3) Again in MailNews context menu, Select All e Reply All both use A as their
accesskey...

On the positive side: I have block images accesskey hooked up and working in the
browser context menu...

Last, I didn't understand the MathML issue: should I add an entity for it (apart
from the Partial Selection one already in) and share the same accesskey with
View Page Source (are they mutually exclusive?)?

Comment 23

[Giacomo Magnini]

Attachment: This should be it

Comment 24

[[e-mail]]

When I originally made the changes I didn't refer to the existing context menu,
and I discovered some keys that might not need changing:
View MathML Source could stay using E instead of U. I'm unsure which is better.
Save Frame could stay using M but probably works better using F.
Unblock Image could stay using U instead of K.
Fit Image to Window could stay using F instead of I. N.B. You can actually
trigger Search Web For on a standalone image, but it makes no sense (no alt text).
View Page Info could stay using I and View Background Image could stay using W.
Save Link Target could stay using R and Set As Wallpaper could stay using S. But
I think more people (especially on Mac/Linux!) would use Save Link Target.

Comment 25

[Ian Neal]

As far as I can see in mailnews you don't get Block/Unblock at the same as Mark
so there shouldn't be a clash with 'k'
I would say 'e' was better for View MathML Source

Comment 26

[Giacomo Magnini]

(In reply to comment #25)
> As far as I can see in mailnews you don't get Block/Unblock at the same as Mark
> so there shouldn't be a clash with 'k'

That's because the accesskeys are not linked as of today. But if you add them to
the xul file for mailnews context menu, the problem will be there. And I guess
they were not linked just for this reason.

Comment 27

[Ian Neal]

Even with that accesskey for "Block/Unblock", the context menu does not have
"Block/Unblock" in at the same time as "Mark"

Comment 28

[Giacomo Magnini]

Attachment: Reverted some changes, added accesskeys for (un)block images in mailnews message pane context menu

Modified patch per comment 24, 25 and 27.
If anybody feels this should be in 1.8b1, please update the flags accordingly.

Comment 29

[Ian Neal]

Comment on attachment 174295 [details] [diff] [review]
Reverted some changes, added accesskeys for (un)block images in mailnews message pane context menu

>Index: xpfe/communicator/resources/locale/en-US/contentAreaCommands.dtd
>===================================================================
> <!ENTITY setWallpaperCmd.label        "Set As Wallpaper">
>-<!ENTITY setWallpaperCmd.accesskey    "S">
>+<!ENTITY setWallpaperCmd.accesskey    "r">
.
.
> <!ENTITY saveLinkAsCmd.label          "Save Link Target As...">
> <!ENTITY saveLinkCmd.label            "Save Link Target">
>-<!ENTITY saveLinkCmd.accesskey        "r">
>+<!ENTITY saveLinkCmd.accesskey        "S">

You do not need to make the above changes (I think)

Comment 30

[Giacomo Magnini]

I was addressing Neil's impression in comment 24...
If you think this is not needed, then I'll provide another patch before checkin.

Comment 31

[Ian Neal]

Comment on attachment 174295 [details] [diff] [review]
Reverted some changes, added accesskeys for (un)block images in mailnews message pane context menu

I only review on help, so setting back to mvl, not sure why I didn't get a
mid-air collision with Neil

Comment 32

[Ian Neal]

(In reply to comment #30)
> I was addressing Neil's impression in comment 24...
> If you think this is not needed, then I'll provide another patch before checkin.

I'm happy whichever way, mvl and Neil will be the ones doing the reviews.

Comment 33

[Michiel van Leeuwen (email: mvl+moz@)]

(In reply to comment #29)
> >-<!ENTITY setWallpaperCmd.accesskey    "S">
> >+<!ENTITY setWallpaperCmd.accesskey    "r">
> >-<!ENTITY saveLinkCmd.accesskey        "r">
> >+<!ENTITY saveLinkCmd.accesskey        "S">
> You do not need to make the above changes (I think)

They seem to make sense to me. Set as wallpaper is used, ehm, never. save link
target is used a lot. So let that have a more logical accesskey.

Comment 34

[Michiel van Leeuwen (email: mvl+moz@)]

will the U from unblock even conflict with view mathml source? Maybe the easiest
solution would be to make it also use 'k'. That will never conflict with block
images. Only one of those will be visible.

Comment 35

[Giacomo Magnini]

(In reply to comment #34)
> will the U from unblock even conflict with view mathml source? Maybe the easiest
> solution would be to make it also use 'k'. That will never conflict with block
> images. Only one of those will be visible.

They should never appear together (seems so reading all of the comments and
especially comment 24).

Comment 36

[[e-mail]]

Comment on attachment 174295 [details] [diff] [review]
Reverted some changes, added accesskeys for (un)block images in mailnews message pane context menu

>-<!ENTITY search.accesskey             "W">
>+<!ENTITY search.accesskey             "F">
The F is actually lower case in the text (in the .properties file). I don't
mind which one you change ;-)

Comment 37

[Giacomo Magnini]

mvl: anything I can do to get your r+? It would be nice to have this fixed for
1.8b2...

Comment 38

[Michiel van Leeuwen (email: mvl+moz@)]

(In reply to comment #37)
> mvl: anything I can do to get your r+? It would be nice to have this fixed for
> 1.8b2...

First of all, i'm not a module peer or anything. Why pick me for review?
And second, changing a lot of accesskeys doesn't sound like a good idea for a
beta to me.
Third, why does all those keys have to be changed for just one conflict?

Comment 39

[[e-mail]]

Comment on attachment 174295 [details] [diff] [review]
Reverted some changes, added accesskeys for (un)block images in mailnews message pane context menu

>"Bookmark This Page" is not available with an image
Actually, it is :-[

Comment 40

[[e-mail]]

A	Select All
C	Copy
E	Copy Email Address
G	Save Image
H	This Frame
K	(Un)Block Image
L	Copy Link Location
M	Bookmark This Page
O	Properties

B	Back
B	Bookmark This Link

D	Switch Page Direction
D	Delete

F	Forward
F	Search Web For …

I	View Page Info
I	Copy Image
"View Page Info" is not available on images

P	Paste
P	Save Page

R	Redo
R	Reload
R	Set As Wallpaper

S	Switch Text Direction
S	Save Link Target
S	Stop

T	Cut
T	Open Link in New Tab

U	View MathML Source
U	Undo
U	View Selection Source
"View MathML Source" is not available with a selection.
"Undo" is only available on text fields.

V	View Image
V	View Page Source
"View Page Source" is not available with images.

W	Fit Image To Window
W	Open Link in New Window
W	View Background Image
"View Background Image" is not available on a link or an image.
"Fit Image To Window" is not available on a link.

So, what have I missed this time?

Comment 41

[[e-mail]]

Redo is no more, I guess it needs to be removed from these context menus too...

Comment 42

[Karsten Düsterloh]

(In reply to comment #41)
> Redo is no more, I guess it needs to be removed from these context menus too...

I already objected to the removal in textboxes (not that it helped much
afterwards :-/), so I object to the respective removal here also. Having an undo
action that can't be undone is just not very logical...

Actually, as a keyboard person, I guess I could live with a removed menuitem as
long as the keyboard shortcut is still there, but this would impose quite an
inconsistency for mouse users. I don't think that such a basic item should be
sacrificed upon the altar of minimization - it's one the suite's advantages that
in case of need a specific function is _just_ _there_!

Comment 43

[Giacomo Magnini]

Attachment: This should implement Neil's list. No test done (yet) for more duplicates/errors.

Comment 44

[Giacomo Magnini]

Tests done so far with the patch in the message pane:
- Context menu on msg body: Select _A_ll & Reply _A_ll (other used AKs: RFMCLKSPD)
- Context menu on a link: Ok
- Context menu on attached image: Ok
- Context menu on text selection: same prob as the first, plus _C_opy & _C_opy To (other used AKs: RFMLKSPD)

More tests as I prepare some example mails.

Comment 45

[Serge Gautherie (:sgautherie)]

Filter "spam" on "guifeatures-nobody-20080610".

Comment 46

[Philip Chee]

The specific issue in Comment 0 is WORKSFORME. Closing ancient bug. Any remaining access key clashes, please file a new bug.