Closed Bug 639694 Opened 9 years ago Closed 9 years ago

Move EditModeToolbar from editorOverlay

Categories

(SeaMonkey :: Composer, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1b3

People

(Reporter: iann_bugzilla, Assigned: iann_bugzilla)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

EditModeToolbar is only used by editor.xul so should be moved from editorOverlay.xul/dtd and removed from TB's forked editorOverlay.xul
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).
Attachment #517610 - Flags: review?(neil)
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).
Attachment #517610 - Flags: review?(neil)
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.
Attachment #517610 - Attachment is obsolete: true
Attachment #517744 - Flags: review?(neil)
Attachment #517744 - Flags: review?(neil) → review+
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
Attachment #517744 - Flags: review?(bugzilla)
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.
Attachment #517744 - Flags: review?(bugzilla) → review+
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
Attachment #517744 - Attachment description: Move EditModeToolbar code from editorOverlay files v1.1 → Move EditModeToolbar code from editorOverlay files v1.1 [Checked in: Comment 6]
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b3
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
Attachment #517744 - Attachment description: Move EditModeToolbar code from editorOverlay files v1.1 [Checked in: Comment 6] → Move EditModeToolbar code from editorOverlay files v1.1 [Checked in: Comment 6 & Comment 7]
You need to log in before you can comment on or make changes to this bug.