1289794 - Integrate consoleoverlay files

Status: RESOLVED FIXED

(SeaMonkey :: General, enhancement)

Description

[Frank-Rainer Grahl (:frg)]

Bug 1282286 moved the old error console to suite. 

The files suite\common\consoleOverlay.xul and suite\locales\en-US\chrome\common\consoleOverlay.dtd could now be integrated into the new code and then be removed.

Comment 1

[Frank-Rainer Grahl (:frg)]

Attachment: 1289794-consoleoverlay.patch

The xul got quite a few new items so I completely reformatted it.

Comment 2

[Ian Neal]

Comment on attachment 8778623 [details] [diff] [review]
1289794-consoleoverlay.patch

>+++ b/suite/common/console/console.xul
>   <commandset id="consoleCommands">
>-    <command id="cmd_close" oncommand="closeWindow(true)"/>
>+    <command id="cmd_close"
>+             oncommand="closeWindow(true)"/>
Nit: This change is not needed, if all attributes can fit in a single line (less than 80 characters), it is acceptable formatting.

>+    <commandset id="tasksCommands"/>
I'd prefer this at the top of the parent <commandset>

>+    <key id="key_focus1"
>+         key="&focus1.commandkey;"
>+         modifiers="accel"
>          oncommand="gTextBoxEval.focus()"/>
If you're changing the lines around it, you might as well put a ; after the )

>+    <key id="key_focus2"
>+         key="&focus2.commandkey;"
>+         modifiers="alt"
>          oncommand="gTextBoxEval.focus()"/>
If you're changing the lines around it, you might as well put a ; after the )

>+    <keyset id="tasksKeys"/>
I'd prefer this at the top of the parent <keyset>

>-      <menuitem id="menu_copy_cm" command="cmd_copy"
>-                label="&copyCmd.label;" accesskey="&copyCmd.accesskey;"/>
>+      <menuitem id="menu_copy_cm"
>+                command="cmd_copy"
>+                label="&copyCmd.label;"
>+                accesskey="&copyCmd.accesskey;"/>
Preference is to have id, label then accesskey which leaves, in this case, command being the last attribute.

>+    <menubar id="main-menubar"
>+             class="chromeclass-menubar"
>+             grippytooltiptext="&menuBar.tooltip;"
>+             position="1">
I think position is only needed in overlay files.

>+              <menuitem id="toggleToolbarMode"
>+                        type="checkbox"
>+                        checked="true"
>+                        oncommand="goToggleToolbar('ToolbarMode','toggleToolbarMode');"
>+                        label="&toolbarMode.label;"
>+                        accesskey="&toolbarMode.accesskey;"/>
Nit: have oncommand as the last attribute.
>+              <menuitem id="toggleToolbarEval"
>+                        type="checkbox"
>+                        checked="true"
>+                        oncommand="goToggleToolbar('ToolbarEval','toggleToolbarEval');"
>+                        label="&toolbarEval.label;"
>+                        accesskey="&toolbarEval.accesskey;"/>
Nit: have oncommand as the last attribute.

>+      <toolbarbutton id="Console:clear"
>+                     oncommand="clearConsole();"
>+                     label="&clear.label;"
>+                     accesskey="&clear.accesskey;"/>
Nit: have oncommand as the last attribute.

>+      <textbox id="TextboxEval"
>+               class="toolbar"
>+               value=""
>+               onkeypress="onEvalKeyPress(event)"
>+               flex="1"/>
Nit: have onkeypress as the last attribute and ; after ). flex can be after class.

>+      <toolbarbutton id="ButtonEval"
>+                     label="&evaluate.label;"
>+                     accesskey="&evaluate.accesskey;"
>+                     oncommand="evaluateTypein()"/>
Nit: have ; after ).

>+    <statusbarpanel flex="1"
>+                    pack="start">
Nit: This change is not needed, if all attributes can fit in a single line (less than 80 characters), it is acceptable formatting.

>+      <label value="&filter2.label;"
>+             control="Filter"/>
Nit: This change is not needed, if all attributes can fit in a single line (less than 80 characters), it is acceptable formatting.

>+      <textbox accesskey="&filter2.accesskey;"
>+               type="search"
>+               id="Filter"
>+               oncommand="changeFilter();"/>
Nit: accesskey after id please

r/a=me for those addressed.

Comment 3

[Frank-Rainer Grahl (:frg)]

Attachment: 1289794-consoleoverlay-V2.patch

Patch with nits fixed. Review+ from IanN carried forward.

Comment 4

[Frank-Rainer Grahl (:frg)]

https://hg.mozilla.org/comm-central/rev/5831664662c8