325972 - Accesskeys conflicts/missing in Mail context menu

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Giacomo Magnini]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060205 SeaMonkey/1.5a
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060205 SeaMonkey/1.5a

Get Flagged Messages and Get Selected Messages in the context menu of MailNews are reusing already assigned accesskeys (F as Forward for the first, and M as Move To).


Reproducible: Always

Steps to Reproduce:
1. Select a message
2. Right click to open the context menu
3. Verify the conflicts

Comment 1

[Giacomo Magnini]

Attachment: SHould be enough

Can't used d as it is used by Delete Message (for POP3 at least).

Comment 2

[Ian Neal]

Print Preview seems to be missing an accesskey...

Comment 3

[Giacomo Magnini]

(In reply to comment #2)
> Print Preview seems to be missing an accesskey...
> 

 <!ENTITY printPreviewCmd.label "Print Preview">
 <!ENTITY printPreviewCmd.accesskey "v">
 <!ENTITY printPreviewCmd.key "v">

Something fishy going on here...

Comment 4

[Ian Neal]

(In reply to comment #3)
> (In reply to comment #2)
> > Print Preview seems to be missing an accesskey...
> > 
> 
>  <!ENTITY printPreviewCmd.label "Print Preview">
>  <!ENTITY printPreviewCmd.accesskey "v">
>  <!ENTITY printPreviewCmd.key "v">
> 
> Something fishy going on here...
> 
That is the context menu needs the accesskey adding to it for print preview.

Comment 5

[Giacomo Magnini]

Attachment: Add accesskey to Print Preview (mail context menu)

Was not so easy to trace: you'll be surprised at how many "Print Preview" entities there are, even unused ones (like in editor)... Ian, I hate you! :-P

Comment 6

[Karsten Düsterloh]

Comment on attachment 215500 [details] [diff] [review]
Add accesskey to Print Preview (mail context menu)

First of all, the context menu shows different items when invoked from the threadpane and the messagepane - Get Flagged Messages etc. are only visible in the threadpane context menu... You should probably correct the bug title.

Print Preview is shown in both menus, but your patch here only fixes the threadpane context and not the messagepane context. :(

OTOH, the messagepane context still has accesskey conflicts, eg. Select _A_ll/Reply to _A_ll. I'd like to see that fixed, too, while you're at it. :)

For me, this would work:
  _R_eply to Sender Only -> Reply to Sender _O_nly
  Reply to _A_ll         -> _R_eply to All
The rest looks fine.

Comment 7

[Giacomo Magnini]

Attachment: What Mnyromyr said.

Comment 8

[Karsten Düsterloh]

Comment on attachment 215615 [details] [diff] [review]
What Mnyromyr said.

Okay.
I found some more, possibly for a new diff:
- if you have "View->Show Attachments Inline" enabled, the context menu of eg an image lacks an accesskey for "Block Images from this Server"
- the attachment box context mnenu lacks accesskeys for "Detach ..." and "Delete", furthermore, the space before the ... needs to go away

Comment 9

[Giacomo Magnini]

(In reply to comment #8)
> I found some more, possibly for a new diff:

How nice of you! :-P

> - if you have "View->Show Attachments Inline" enabled, the context menu of eg
> an image lacks an accesskey for "Block Images from this Server"

That was actually part of bug 146825... And there you'll find more accesskey dupes, too.

> - the attachment box context mnenu lacks accesskeys for "Detach ..." and
> "Delete", furthermore, the space before the ... needs to go away

Will have a new diff for this. SHould I use a different bug?

Comment 10

[Karsten Düsterloh]

> That was actually part of bug 146825... And there you'll find more accesskey
> dupes, too.

Ah, okay.

> > - the attachment box context mnenu lacks accesskeys for "Detach ..." and
> > "Delete", furthermore, the space before the ... needs to go away
> 
> Will have a new diff for this. SHould I use a different bug?

Not necessarily, this bug's topic is broad enough by now. ;-)
But I vaguely remember that the Detach etc. stuff has been mentioned somewhere else...

Comment 11

[[e-mail]]

Comment on attachment 215615 [details] [diff] [review]
What Mnyromyr said.

> <!ENTITY downloadSelectedCmd.label "Get Selected Messages">
>-<!ENTITY downloadSelectedCmd.accesskey "M">
>+<!ENTITY downloadSelectedCmd.accesskey "G">
> <!ENTITY downloadFlaggedCmd.label "Get Flagged Messages">
>-<!ENTITY downloadFlaggedCmd.accesskey "F">
>+<!ENTITY downloadFlaggedCmd.accesskey "t">
I sorta feel that these access keys should be the other way around (there are two 't's in "Get Selected Messages").

Also, should we sync these access keys to the main menus?

Comment 12

[Giacomo Magnini]

Attachment: Fix for Detach problem

Comment 13

[Giacomo Magnini]

Attachment: What Neil suggested, even for the File menu.

Comment 14

[[e-mail]]

Comment on attachment 216088 [details] [diff] [review]
What Neil suggested, even for the File menu. 

Sorry, I misunderstood the previous patch and got you to correct something that wasn't wrong...

Comment 15

[[e-mail]]

Comment on attachment 215615 [details] [diff] [review]
What Mnyromyr said.

> <!ENTITY downloadSelectedCmd.label "Get Selected Messages">
>-<!ENTITY downloadSelectedCmd.accesskey "M">
>+<!ENTITY downloadSelectedCmd.accesskey "G">
> <!ENTITY downloadFlaggedCmd.label "Get Flagged Messages">
>-<!ENTITY downloadFlaggedCmd.accesskey "F">
>+<!ENTITY downloadFlaggedCmd.accesskey "t">
And indeed there are no fewer than four "g"s in "Get Flagged Messages" ;-)
sr=me with the G & T switched.
Sorry for not realising that these already affect the Offline menu.

Comment 16

[Giacomo Magnini]

Attachment: Reverted the unwanted changes, but left the swapped accesskeys, as requested.

Carrying over r/sr. Can someone please check this in?