Move EditModeToolbar from editorOverlay

RESOLVED FIXED in seamonkey2.1b3

Status

SeaMonkey
Composer
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Ian Neal, Assigned: Ian Neal)

Tracking

(Blocks: 1 bug)

Trunk
seamonkey2.1b3
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
EditModeToolbar is only used by editor.xul so should be moved from editorOverlay.xul/dtd and removed from TB's forked editorOverlay.xul
(Assignee)

Comment 1

7 years ago
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).
Attachment #517610 - Flags: review?(neil)

Comment 2

7 years ago
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).
(Assignee)

Updated

7 years ago
Attachment #517610 - Flags: review?(neil)
(Assignee)

Comment 3

7 years ago
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.
Attachment #517610 - Attachment is obsolete: true
Attachment #517744 - Flags: review?(neil)

Updated

7 years ago
Attachment #517744 - Flags: review?(neil) → review+
(Assignee)

Comment 4

7 years ago
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+
(Assignee)

Comment 6

7 years ago
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]
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b3
(Assignee)

Comment 7

7 years ago
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.