Closed Bug 146825 Opened 22 years ago Closed 14 years ago

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

Categories

(SeaMonkey :: UI Design, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: olgam, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file, 5 obsolete files)

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)
Keywords: access, nsbeta1
-> Xp APPS
Assignee: Matti → blaker
Component: Browser-General → XP Apps: GUI Features
QA Contact: imajes-qa → paw
qa  --> sairuh@netscape.com
QA Contact: paw → sairuh
Nav triage team: nsbeta1-
Keywords: nsbeta1nsbeta1-
could this be a dup of bug 135225?
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.
Target Milestone: --- → Future
nominating for buffy...
Keywords: nsbeta1-nsbeta1
Nav triage team: nsbeta1-
Keywords: nsbeta1nsbeta1-
Summary: 2 'w' in context menu conflict - for a link while some text is selected → 2 'w' accesskeys in context menu conflict - for a link while some text is selected
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.
Assignee: firefox → guifeatures
Target Milestone: Future → ---
Blocks: accesskey
Product: Core → Mozilla Application Suite
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.
Attachment #171619 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #171619 - Flags: superreview-
Attachment #171619 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #171619 - Flags: review-
(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.
Let's just hope that nobody will add an accesskey for Block images from this
server, or we gonna have some real troubles... :-)
Attachment #171619 - Attachment is obsolete: true
Attachment #171630 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #171630 - Flags: review?(neil.parkwaycc.co.uk)
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?
Attached patch Patch for Neil's suggestion (obsolete) — Splinter Review
I don't seem to find the accesskey for View MathML Source...
Attachment #171630 - Attachment is obsolete: true
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...
(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.
Will Send Page... (accesskey = d) clash with Switch Page Direction?
Maybe we can use 'k' as the accesskey for Block/Unblock images from this server?
(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.
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.
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?)?
Attached patch This should be it (obsolete) — Splinter Review
Attachment #172088 - Attachment is obsolete: true
Attachment #172662 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #172662 - Flags: review?(neil.parkwaycc.co.uk)
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.
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
(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.
Even with that accesskey for "Block/Unblock", the context menu does not have
"Block/Unblock" in at the same time as "Mark"
Modified patch per comment 24, 25 and 27.
If anybody feels this should be in 1.8b1, please update the flags accordingly.
Attachment #172662 - Attachment is obsolete: true
Attachment #174295 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #174295 - Flags: review?(bugzilla)
Attachment #171630 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #171630 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #172662 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #172662 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #174295 - Flags: review?(bugzilla) → review?(mvl)
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)
Attachment #174295 - Flags: review?(mvl) → review?(bugzilla)
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 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
Attachment #174295 - Flags: review?(bugzilla) → review?(mvl)
(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.
(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.
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.
(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 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 ;-)
Attachment #174295 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
mvl: anything I can do to get your r+? It would be nice to have this fixed for
1.8b2...
(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 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 :-[
Attachment #174295 - Flags: review?(mvl) → review-
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?
Redo is no more, I guess it needs to be removed from these context menus too...
(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_!
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.
Filter "spam" on "guifeatures-nobody-20080610".
Assignee: guifeatures → nobody
QA Contact: bugzilla → guifeatures
Component: XP Apps: GUI Features → UI Design
The specific issue in Comment 0 is WORKSFORME. Closing ancient bug. Any remaining access key clashes, please file a new bug.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: