675607 - Make Composer use utilityOverlay's key_close and menu_close

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Ian Neal]

Attachment: Deduplicate close and switch Composer

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.

Comment 1

[[e-mail]]

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.

Comment 2

[Ian Neal]

Attachment: Switch composer to use utilityOverlay [Checked in: Comment 5]

Changes since last version:
* Revert platform overlay changes.
* Re-arrange order of close and quit for key and command elements.

Comment 3

[[e-mail]]

(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 4

[Ian Neal]

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.

Comment 5

[Ian Neal]

Comment on attachment 550348 [details] [diff] [review]
Switch composer to use utilityOverlay [Checked in: Comment 5]

http://hg.mozilla.org/comm-central/rev/1a19ffe0f5de