Closed Bug 675607 Opened 8 years ago Closed 8 years ago

Make Composer use utilityOverlay's key_close and menu_close

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set

Tracking

(seamonkey2.5 fixed)

RESOLVED FIXED
seamonkey2.5
Tracking Status
seamonkey2.5 --- fixed

People

(Reporter: iann_bugzilla, Assigned: iann_bugzilla)

Details

Attachments

(1 file, 1 obsolete file)

At the moment Composer has its own closekb and menu_close elements but it could make use of key_close and menu_close provided by utilityOverlay and platformCommunicatorOverlay.
There is also some code to do with closing that is duplicated across the platformCommunicatorOverlay files that could be simplified by moving into utilityOverlay.
This patch:
* Moves the duplicated code out of the platformCommunicatorOverlay files into utilityOverlay
* Switches Composer's editorOverlay to use key_close and menu_close from utilityOverlay.
Attachment #549768 - Flags: review?(neil)
Comment on attachment 549768 [details] [diff] [review]
Deduplicate close and switch Composer

>+    <key id="key_close"/>
>     <key id="printkb"
>          key="&printCmd.key;"
>          command="cmd_print"
>          modifiers="accel"/>
>     <key id="key_quit"/>
Nit: key_close would look nicer moved to just before key_quit.

>-    <command id="cmd_close"        label="&closeCmd.label;"         oncommand="goDoCommand('cmd_close')"/>
>+    <command id="cmd_close" oncommand="goDoCommand('cmd_close');"/>
>     <command id="cmd_printSetup"   oncommand="goDoCommand('cmd_printSetup')"/>
>     <command id="cmd_print"        oncommand="goDoCommand('cmd_print')"/>
>     <command id="cmd_quit"         oncommand="goDoCommand('cmd_quit')"/>
Ditto, but also align the oncommand please.

>-  <menuitem id="menu_close" label="&closeCmd.label;" key="key_close" accesskey="&closeCmd.accesskey;" command="cmd_close"/>
>-  <key id="key_close"  key="&closeCmd.key;" command="cmd_close" modifiers="accel"/>
>-  <key id="key_closeWindow"  key="&closeCmd.key;" command="cmd_closeWindow" modifiers="accel,shift"/>
>+  <menuitem id="menu_close" accesskey="&closeCmd.accesskey;"/>
I'm not keen on moving the menuitem, as you need to overlay it again anyway. I don't mind about the keys though.

>-<!ENTITY closeCmd.label                 "Close">  
>-<!ENTITY closeCmd.key                   "W">  
>-<!ENTITY closeCmd.accesskey             "c">
>+<!ENTITY closeCmd.accesskey             "C">
Also splitting the entities across files is a bad idea if you can avoid it, I think we're better off just leaving them in the platform DTD files for now.
Summary: Simplify key_close, key_closeWindow and menu_close and make Composer use it → Make Composer use utilityOverlay's key_close and menu_close
Changes since last version:
* Revert platform overlay changes.
* Re-arrange order of close and quit for key and command elements.
Attachment #549768 - Attachment is obsolete: true
Attachment #549768 - Flags: review?(neil)
Attachment #550348 - Flags: review?(neil)
Attachment #550348 - Flags: review?(neil) → review+
(From update of attachment 549768 [details] [diff] [review])
>-    <command id="cmd_close"        label="&closeCmd.label;"         oncommand="goDoCommand('cmd_close')"/>
>+    <command id="cmd_close" oncommand="goDoCommand('cmd_close');"/>
The second change was too subtle for me to spot the first time ;-)
Comment on attachment 550348 [details] [diff] [review]
Switch composer to use utilityOverlay [Checked in: Comment 5]

Requesting additional review for the dtd entity removals, they do not appear to be used by TB but it is a shared file.
Attachment #550348 - Flags: review?(mbanner)
Attachment #550348 - Flags: review?(mbanner) → review+
Comment on attachment 550348 [details] [diff] [review]
Switch composer to use utilityOverlay [Checked in: Comment 5]

http://hg.mozilla.org/comm-central/rev/1a19ffe0f5de
Attachment #550348 - Attachment description: Switch composer to use utilityOverlay → Switch composer to use utilityOverlay [Checked in: Comment 5]
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.5
You need to log in before you can comment on or make changes to this bug.