Closed Bug 250005 Opened 20 years ago Closed 20 years ago

Shortcut keys conflict: New Message and Mark All Read are both Command+Shift+M

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: smjg, Assigned: pkwarren)

References

Details

(Keywords: polish, regression)

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a2) Gecko/20040705
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a2) Gecko/20040705

First I discovered that Cmd+Shift+C had stopped working to mark all messages in
a newsgroup as read.  I then discovered that it had been changed to Cmd+Shift+M
(bug 186789).  However, this shortcut doesn't work either.

The problem is that two commands have become assigned the same shortcut key:
Message -> New Message and Message -> Mark -> All Read.

Reproducible: Always
Steps to Reproduce:
1. Open a newsgroup.
2. Press Cmd+Shift+M.

Actual Results:  
A message composition window appears.

Expected Results:  
Well, I was expecting it to mark all messages in the newsgroup as read.  We need
to change one of the shortcut keys so that it doesn't conflict.
CCing the owner of bug 186789. See also bug 186789 comment 70.
Thinking about it, we should keep Cmd+Shift+M for New Message and change Mark
All Read.  That way, people who use Cmd+Shift+M for New Message won't suddenly
find their messages inadvertently being marked read when this is fixed.

OTOH nobody uses Cmd+Shift+M for Mark All Read, since it doesn't work, so giving
this command a new shortcut won't interfere with anything.
*** Bug 252430 has been marked as a duplicate of this bug. ***
Component: Mail Window Front End → Keyboard: Navigation
Product: MailNews → Browser
Assignee: sspitzer → aaronleventhal
Keywords: regression
Flags: blocking-aviary1.0mac?
Assignee: aaronleventhal → pkwarren
Guys- up until 1.8a1 this worked!  It's a regression.
I would prefer to use the same keyboard shortcut on all platforms for Mark All
Read, and unfortunately both the old shortcut (Ctrl+Shift+C) and the Outlook
Express shortcut (Ctrl+Shift+A) will not work in GTK2 builds (Bug 186789), so we
need to choose a new shortcut from the following list:

http://www.mozilla.org/projects/ui/accessibility/mozkeylist.html
Attached patch Patch v1 (obsolete) — Splinter Review
This changes the shortcut for Mark All Read back to Ctrl+Shift+C for Mac/Win
and only changes it for Linux builds. This is one alternative to defining a new
shortcut on all platforms. Neil - what do you think?
Attachment #154044 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 154044 [details] [diff] [review]
Patch v1

I don't think we're allowed to have unlocalizable key bindings.
Attachment #154044 - Flags: review?(neil.parkwaycc.co.uk) → review-
While fixing this, please also change in
extensions/help/resources/locale/en-US/shortcuts-mailnews.xhtml.
Attached patch Patch v2Splinter Review
I have added a new entity to messenger.dtd, as there is not a preexisting
platform specific dtd file in MailNews. I have also updated the shortcuts help
section to include the changes.
Attachment #154044 - Attachment is obsolete: true
Attachment #154496 - Flags: review?(neil.parkwaycc.co.uk)
*** Bug 253432 has been marked as a duplicate of this bug. ***
Keywords: polish
Attachment #154496 - Flags: review?(neil.parkwaycc.co.uk) → review+
Attachment #154496 - Flags: superreview?(mscott)
What is the status of this patch?  What needs to be done to get it landed?  Can
I help?

Thanks...
Comment on attachment 154496 [details] [diff] [review]
Patch v2

David - Can you super review?
Attachment #154496 - Flags: superreview?(mscott) → superreview?(bienvenu)
Attachment #154496 - Flags: superreview?(bienvenu) → superreview+
Comment on attachment 154496 [details] [diff] [review]
Patch v2

Requesting approval for a simple patch which fixes a very visible regression in
Mail/News on Mac.
Attachment #154496 - Flags: approval1.8a3?
Attachment #154496 - Flags: approval1.7.3?
Attachment #154496 - Flags: approval-aviary?
Attachment #154496 - Flags: approval1.7.3?
Comment on attachment 154496 [details] [diff] [review]
Patch v2

a=mkaply
Attachment #154496 - Flags: approval1.8a3?
Attachment #154496 - Flags: approval1.8a3+
Attachment #154496 - Flags: approval-aviary?
Attachment #154496 - Flags: approval-aviary+
I see this patch has been nominated for the aviary 1.0 branch but I don't see
that it contains any changes for aviary files. Does this bug exist on the aviary
branch? And if so, you'll need a patch that modifies the files actually used by
the aviary branch instead of the mailnews equivalents. 
Fixed on trunk.

Checking in extensions/help/resources/locale/en-US/shortcuts-mailnews.xhtml;
/cvsroot/mozilla/extensions/help/resources/locale/en-US/shortcuts-mailnews.xhtml,v
 <--  shortcuts-mailnews.xhtml
new revision: 1.8; previous revision: 1.7
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.7; previous revision: 1.6
done
Checking in mailnews/base/resources/locale/en-US/messenger.dtd;
/cvsroot/mozilla/mailnews/base/resources/locale/en-US/messenger.dtd,v  <-- 
messenger.dtd
new revision: 1.191; previous revision: 1.190
done
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
(In reply to comment #15)
> I see this patch has been nominated for the aviary 1.0 branch but I don't see
> that it contains any changes for aviary files. Does this bug exist on the aviary
> branch? And if so, you'll need a patch that modifies the files actually used by
> the aviary branch instead of the mailnews equivalents. 

You are correct - the patch for Bug 186789 (which caused this regression) was
never checked in on the Aviary branch.
Flags: blocking-aviary1.0mac?
(In reply to comment #17)
> (In reply to comment #15)
> > I see this patch has been nominated for the aviary 1.0 branch but I don't see
> > that it contains any changes for aviary files. Does this bug exist on the aviary
> > branch? And if so, you'll need a patch that modifies the files actually used by
> > the aviary branch instead of the mailnews equivalents. 
> 
> You are correct - the patch for Bug 186789 (which caused this regression) was
> never checked in on the Aviary branch.


So do BOTH then get checked in? Just curious how that would work, and why it was
missed the first time around.
(In reply to comment #18)
> So do BOTH then get checked in? Just curious how that would work, and why it was
> missed the first time around.

The patch to fix Bug 186789 only fixes the problem in the SeaMonkey Mail/News
client. If someone is interested in changing the keybindings to not conflict in
GTK2 Thunderbird builds, a new bug should be opened for that.
Thanks for a prompt (by Mozilla standards) fix to this bug!  Now, if only other
bugs could be such quick fixes....
Status: RESOLVED → VERIFIED
Re: comment #19

The corresponding Thunderbird issues were bug 239483 (Mark All Read
Ctrl+Shift+C), bug 242947 (Search Mail Ctrl+Shift+F), and bug 269228 (Select
Thread Ctrl+Shift+A).  The first two were resolved by changing them to Shift+C
and Shift+F respectively on GTK2 builds.  The last was missed, but I've attached
a corresponding patch to that bug to change it to Shift+A.
Component: Keyboard: Navigation → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: