410164 - Put back commandkeys without modifiers in Go/Message submenus

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Stefan [:stefanh]]

Optionally, someone should confirm that bug 195979 is fixed, but it seems to work now...

Comment 1

[Stefan [:stefanh]]

Attachment: Display keys in Mac menus

Basically a back-out of the patch in bug 194147.

Comment 2

[Stefan [:stefanh]]

JFTR: Phil's Thunderbird version is in bug 199019.

Comment 3

[Karsten Düsterloh]

Comment on attachment 294831 [details] [diff] [review]
Display keys in Mac menus

>Index: mailnews/base/resources/content/mailWindowOverlay.xul
>===================================================================
>         <menuitem id="markReadMenuItem"
>         <menuitem id="markFlaggedMenuItem"

Why didn't you remove these ids, too?

But actually, every menuitem should have an id (eg to ease extension development), so please don't remove them at all.

Comment 4

[[e-mail]]

Comment on attachment 294831 [details] [diff] [review]
Display keys in Mac menus

Also, can you please not put the key between the label and its accesskey, thanks.

Comment 5

[Stefan [:stefanh]]

(In reply to comment #3)
> (From update of attachment 294831 [details] [diff] [review])
> >Index: mailnews/base/resources/content/mailWindowOverlay.xul
> >===================================================================
> >         <menuitem id="markReadMenuItem"
> >         <menuitem id="markFlaggedMenuItem"
> 
> Why didn't you remove these ids, too?

I guess I never checked why they where there before the patch in bug 194147.

> But actually, every menuitem should have an id (eg to ease extension
> development), so please don't remove them at all.

OK. 

(In reply to comment #4)
> (From update of attachment 294831 [details] [diff] [review])
> Also, can you please not put the key between the label and its accesskey,
> thanks.

Oops, I followed the bad examples in the file.

Comment 6

[Stefan [:stefanh]]

Attachment: New version

OK, keys are now after accesskeys. All id's are back and I also added two id's in the markMenu.

Comment 7

[Karsten Düsterloh]

Comment on attachment 294956 [details] [diff] [review]
New version

Just two minor nits for check-in: ;-)

>Index: mailnews/base/resources/content/mailWindowOverlay.xul
>===================================================================
>       <menu id="goNextMenu" label="&nextMenu.label;" accesskey="&nextMenu.accesskey;">
>       <menu id="goPreviousMenu" label="&prevMenu.label;" accesskey="&prevMenu.accesskey;">

Please use the one-attribute-per-line style for their menuitems (as used for later ones) and add the ids nextFlaggedMenuItem/prevFlaggedMenuItem while you're at it.

r=me with that.

Comment 8

[Stefan [:stefanh]]

Landed with Karsten's nits addressed:
Checking in mailnews/base/resources/content/mailWindowOverlay.xul;
/cvsroot/mozilla/mailnews/base/resources/content/mailWindowOverlay.xul,v  <--  mailWindowOverlay.xul
new revision: 1.344; previous revision: 1.343
done
Checking in mailnews/base/resources/content/unix/platformMailnewsOverlay.xul;
/cvsroot/mozilla/mailnews/base/resources/content/unix/platformMailnewsOverlay.xul,v  <--  platformMailnewsOverlay.xul
new revision: 1.10; previous revision: 1.9
done
Checking in mailnews/base/resources/content/win/platformMailnewsOverlay.xul;
/cvsroot/mozilla/mailnews/base/resources/content/win/platformMailnewsOverlay.xul,v  <--  platformMailnewsOverlay.xul
new revision: 1.7; previous revision: 1.6
done