Last Comment Bug 639694 - Move EditModeToolbar from editorOverlay
: Move EditModeToolbar from editorOverlay
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Composer (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1b3
Assigned To: Ian Neal
:
Mentors:
Depends on:
Blocks: editorOverlaytidy
  Show dependency treegraph
 
Reported: 2011-03-07 17:45 PST by Ian Neal
Modified: 2011-03-11 18:01 PST (History)
2 users (show)
iann_bugzilla: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Move EditModeToolbar code from editorOverlay files v1.0 (17.54 KB, patch)
2011-03-07 18:08 PST, Ian Neal
no flags Details | Diff | Splinter Review
Move EditModeToolbar code from editorOverlay files v1.1 [Checked in: Comment 6 & Comment 7] (17.26 KB, patch)
2011-03-08 07:55 PST, Ian Neal
neil: review+
standard8: review+
Details | Diff | Splinter Review

Description Ian Neal 2011-03-07 17:45:27 PST
EditModeToolbar is only used by editor.xul so should be moved from editorOverlay.xul/dtd and removed from TB's forked editorOverlay.xul
Comment 1 Ian Neal 2011-03-07 18:08:26 PST
Created attachment 517610 [details] [diff] [review]
Move EditModeToolbar code from editorOverlay files v1.0

This patch:
* Moves EditModeToolbar xul from editorOverlay.xul to editor.xul
* Removes EditModeToolbar xul from TB's forked editorOverlay.xul
* Moves EditModeToolbar entities from editorOverlay.dtd to editor.dtd (and corrects the cases for the accesskeys).
Comment 2 neil@parkwaycc.co.uk 2011-03-08 01:50:44 PST
Comment on attachment 517610 [details] [diff] [review]
Move EditModeToolbar code from editorOverlay files v1.0

>+                observes="cmd_NormalMode"/>
Prefer command="cmd_NormalMode" instead (assuming it works...)

>+    <hbox id="EditModeToolbar"
>+          align="center"
>+          hidden="false"
[I've no idea why we set align and hidden here]

>+        <tab id="NormalModeButton"
>+             class="tab-bottom edit-mode _plain"
[I've no idea what _plain does]

>+             type="text"
[Or type]

>+             selected="1"
[This has no effect; they may have meant selected="true" but that's the default for the first tab anyway.]

>+             oncommand="goDoCommand('cmd_NormalMode');"/>
Consider command="cmd_NormalMode" instead

>+      </tabs>
>     </hbox>
> 
>   </vbox> <!-- appcontent -->
> </hbox><!-- sidebar-parent -->
[I've experimented with having the tabs on the outside of the sidebar, but I've forgotten why, and it was probably a bad move anyway...]

>+<!ENTITY NormalModeTab.label "Normal">
>+<!ENTITY NormalMode.label "Normal Edit Mode">
Do you think localisers are going to want different strings in the menu?
Might be worth at least giving them the option, as you're changing things.

>+<!-- Toolbar has an image with "HTML" text in it,
>+     so don't include it in the string -->
>+<!ENTITY SourceMode.label "Source">
IMHO this comment should be rolled in with the L10N note below i.e.
<!-- LOCALIZATION NOTE: (HTMLSourceModeTab.dir, HTMLSourceModeTab.label)
     (stuff about the tab having an image with <HTML> on it
      so you don't need to include HTML in the label but
      you might need to decide to have the image on the right) -->
<!ENTITY HTMLSourceModeTab.dir "">
<!ENTITY HTMLSourceModeTab.label "Source">
<!ENTITY HTMLSourceMode.label "HTML Source">
<!ENTITY HTMLSourceMode.accesskey "H">
Oh, and the dir entity should use a .dir name, of course (not necessarily the one I picked, that was just for example purposes).
Comment 3 Ian Neal 2011-03-08 07:55:59 PST
Created attachment 517744 [details] [diff] [review]
Move EditModeToolbar code from editorOverlay files v1.1 [Checked in: Comment 6 & Comment 7]

Changes since v1.0:
* Moved from using observes/oncommand to using command.
* Removed obsolete code as suggested by reviewer.
* Entities tweaked to give more flexibility to localisers.
Comment 4 Ian Neal 2011-03-08 11:44:35 PST
Comment on attachment 517744 [details] [diff] [review]
Move EditModeToolbar code from editorOverlay files v1.1 [Checked in: Comment 6 & Comment 7]

Requesting review for TB removals
Comment 5 Mark Banner (:standard8) 2011-03-10 04:40:08 PST
Comment on attachment 517744 [details] [diff] [review]
Move EditModeToolbar code from editorOverlay files v1.1 [Checked in: Comment 6 & Comment 7]

This had bitrot on my machine for some reason, but applying the change worked fine.
Comment 6 Ian Neal 2011-03-10 16:54:22 PST
Comment on attachment 517744 [details] [diff] [review]
Move EditModeToolbar code from editorOverlay files v1.1 [Checked in: Comment 6 & Comment 7]

http://hg.mozilla.org/comm-central/rev/25d7a188c85d

Bit rot was due to another patch in my queue which is waiting on review
Comment 7 Ian Neal 2011-03-11 18:01:12 PST
Comment on attachment 517744 [details] [diff] [review]
Move EditModeToolbar code from editorOverlay files v1.1 [Checked in: Comment 6 & Comment 7]

http://hg.mozilla.org/comm-central/rev/8c28537630df

For some reason the previous check in was missing the commandset changes in editor.xul

Note You need to log in before you can comment on or make changes to this bug.