Redo Undo both use keyboard shortcut COMMAND+Z in Mail message composition

RESOLVED FIXED in seamonkey2.1a1

Status

SeaMonkey
MailNews: Composition
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Cecelia S, Assigned: stefanh)

Tracking

({fixed-seamonkey2.0.1})

Trunk
seamonkey2.1a1
All
Mac OS X
fixed-seamonkey2.0.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

1.79 KB, patch
Karsten Düsterloh
: review+
neil@parkwaycc.co.uk
: superreview+
Karsten Düsterloh
: approval-seamonkey2.0.1+
Details | Diff | Splinter Review
(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.23) Gecko/20090823 SeaMonkey/1.1.18 Mnenhy/0.7.5.0
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.23) Gecko/20090823 SeaMonkey/1.1.18 Mnenhy/0.7.5.0

Typical keyboard commands for Undo and Redo (with unlimited Undo) are COMMAND+Z for Undo and COMMAND+SHIFT+Z for Redo. This is the labeling in the browser component of SeaMonkey, but under Mail, the shortcuts are both COMMAND+Z. The COMMAND+Z/COMMAND+SHIFT+Z shortcuts work correctly in the subject line of the email, but not in the message area.

Reproducible: Always

Steps to Reproduce:
1. Compose mail message
2. Make several additions and deletions
3. Press COMMAND+Z a few times
Actual Results:  
Last edit is undone and redone.

Expected Results:  
Last few edits progressively undone.

I've filed the menu keyboard shortcut display as Bug#528628
(Reporter)

Comment 1

8 years ago
I just noticed that my build identifier is wrong. This is under SeaMonkey 2.0 (final release).
(Assignee)

Comment 2

8 years ago
I guess we miss some overlay here...
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 3

8 years ago
Karsten, any idea what caused this? The only file I can see the right key combo in is platformCommunicatorOverlay
(Assignee)

Comment 4

8 years ago
So... we get those keys from utilityOverlay.xul, it seems. That is overlayed by platformCommunicatorOverlay.xul.
(Assignee)

Comment 5

8 years ago
Created attachment 412514 [details] [diff] [review]
possible patch

Not really tested this, just putting it here for reference
(Reporter)

Updated

8 years ago
Version: unspecified → SeaMonkey 2.0 Branch
(Assignee)

Comment 6

8 years ago
OK, so the <key/> in platformCommunicatorOverlay.xul doesn't work at all, but it does if I apply attachment #412514 [details] [diff] [review].
(Assignee)

Updated

8 years ago
Assignee: nobody → stefanh
Status: NEW → ASSIGNED
(Assignee)

Comment 7

8 years ago
Comment on attachment 412514 [details] [diff] [review]
possible patch

I think there is more to do here - it's a bit of a mess with all the .dtd files. But this seems reasonable to me since I believe we want this on branch (no l10n impact).
Attachment #412514 - Flags: superreview?(neil)
Attachment #412514 - Flags: review?(mnyromyr)
(Assignee)

Comment 8

8 years ago
We'll fix this on trunk first
Target Milestone: --- → seamonkey2.1a1
Version: SeaMonkey 2.0 Branch → Trunk
(Assignee)

Updated

8 years ago
Duplicate of this bug: 528628
Comment on attachment 412514 [details] [diff] [review]
possible patch

(As this patch stands, it looks like it breaks Linux builds.)

For some reason the key_redo attributes are being overlayed in the wrong order (i.e. the attributes from the platform overlay are getting applied first) and I have no idea why :s
Attachment #412514 - Flags: superreview?(neil) → superreview-
OK, so the issue seems to be the order of overlays.
messengercompose.xul includes the keyset from editorOverlay.xul that includes the key_redo element that utilityOverlay.xul overlays. However if utilityOverlay.xul is overlaid before editorOverlay.xul then its overlay temporarily misses. Now platformCommunicatorOverlay.xul is an overlay of an overlay, so it gets queued up after all of the main overlays, so that it does find the key_redo from editorOverlay.xul, but then when utilityOverlay.xul gets its second chance to overlay key_redo it overwrites the attributes that platformCommunicatorOverlay.xul set.
(Assignee)

Comment 12

8 years ago
Created attachment 414863 [details] [diff] [review]
New approach

Thanks for the info, Neil.

On mac, the issue can be seen in the main mailnews window as well (see the dupe). Anyway, I switched the order of the overlays and it works fine for me. I think it makes sense to remove the attributes in utilityOverlay, since we're not really using them.
Attachment #412514 - Attachment is obsolete: true
Attachment #414863 - Flags: superreview?(neil)
Attachment #414863 - Flags: review?(mnyromyr)
Attachment #412514 - Flags: review?(mnyromyr)
(Assignee)

Comment 13

8 years ago
(like to keep this o the blocking radar, so it won't get lost)
Flags: blocking-seamonkey2.0.1?
Comment on attachment 414863 [details] [diff] [review]
New approach

>diff --git a/suite/common/utilityOverlay.xul b/suite/common/utilityOverlay.xul
As I said already before, this change breaks Linux!

sr=me on the mailnews changes only.
Attachment #414863 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 15

8 years ago
Created attachment 414869 [details] [diff] [review]
Overlay key for linux

OK, if I haven't miunderstood you completely this would be better since then there won't be any confusion regarding what's overlayed and not. And this shouldn't break linux.
Attachment #414869 - Flags: superreview?(neil)
Attachment #414869 - Flags: review?(mnyromyr)
(Assignee)

Updated

8 years ago
Attachment #414863 - Flags: review?(mnyromyr)
Comment on attachment 414869 [details] [diff] [review]
Overlay key for linux

>+  <key id="key_redo"/>
No point just having this on its own.
(Assignee)

Comment 17

8 years ago
Created attachment 414873 [details] [diff] [review]
Even better

Neil pointed out that I don't need the key in utilityOverlay anymore. This patch also skips the re-ordering of the overlays.
Attachment #414863 - Attachment is obsolete: true
Attachment #414869 - Attachment is obsolete: true
Attachment #414873 - Flags: superreview?(neil)
Attachment #414873 - Flags: review?(mnyromyr)
Attachment #414869 - Flags: superreview?(neil)
Attachment #414869 - Flags: review?(mnyromyr)

Updated

8 years ago
Attachment #414873 - Flags: superreview?(neil) → superreview+

Comment 18

8 years ago
Comment on attachment 414873 [details] [diff] [review]
Even better

Interestingly, shift^H does (and did) redo even under Linux, which I didn't know, becaus e it's not shown...
Attachment #414873 - Flags: review?(mnyromyr) → review+
(Assignee)

Comment 19

8 years ago
http://hg.mozilla.org/comm-central/rev/fb0d8c6fdd8d
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Hardware: PowerPC → All
Resolution: --- → FIXED
(Assignee)

Comment 20

8 years ago
Comment on attachment 414873 [details] [diff] [review]
Even better

I think we want this on 2.0.1 - it's a safe patch and the problem is highly visible for mac users.
Attachment #414873 - Flags: approval-seamonkey2.0.1?

Updated

8 years ago
Attachment #414873 - Flags: approval-seamonkey2.0.1? → approval-seamonkey2.0.1+
(Assignee)

Comment 21

8 years ago
Fixed for the forthcoming 2.0.1 release: http://hg.mozilla.org/releases/comm-1.9.1/rev/eb0b14ffe235
Flags: blocking-seamonkey2.0.1?
Keywords: fixed-seamonkey2.0.1
You need to log in before you can comment on or make changes to this bug.