Closed Bug 680805 Opened 9 years ago Closed 8 years ago

Share edit menus between Composer, Plain Text Editor and Messenger Compose

Categories

(SeaMonkey :: Composer, defect)

defect
Not set

Tracking

(seamonkey2.7 fixed)

RESOLVED FIXED
seamonkey2.7
Tracking Status
seamonkey2.7 --- fixed

People

(Reporter: iann_bugzilla, Assigned: iann_bugzilla)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

Attached patch Share edit menu (obsolete) — Splinter Review
At the moment Messenger Compose has a separate edit menu to the one for Composer and Plain Text Editor.
The plan is to introduce two new overlays:
* editorOnlyOverlay.xul for code that is only used in both Composer and Plain Text Editor and can be moved out of editorOverlay.xul
* mailComposeOverlay.xul for code that needs to be added on top of editorOverlay.xul for messengercompose.xul

This will allow for the edit menu to be shared from editorOverlay.xul for all of Composer, Plain Text Editor and Messenger Compose.

This patch:
* Creates editorOnlyOverlay.xul and moves publish settings xul and inlinespellcheck editor specific xul to it from editorOverlay.xul
* Adds editorOnlyOverlay.xul to editor.xul and debugQATextEditorShell.xul
* Creates editorOnlyOverlay.dtd and moves publish settings entities from editorOverlay.dtd
* Adds ellipse to Check Spelling entity in editorOverlay.dtd that was missing.
* Remove now unneeded entities for find and replace and check/inline spelling from messengercompose.dtd
* Updates references to menuitems in MsgComposeCommands.js
* Moves label/accesskey attributes to commands for rewrap and account manager in messengercompose.xul
* Creates mailComposeOverlay.xul and moves pasteQuote, rewrap, account manager and inlinespellcheck messenger compose specific xul to it from messengercompose.xul
* Adds mailComposeOverlay.xul to messengercompose.xul
* Remove menu_Edit xul and spelling menu xul from messengercompose.xul
Attachment #554762 - Flags: review?(neil)
Comment on attachment 554762 [details] [diff] [review]
Share edit menu

>diff --git a/editor/ui/composer/content/editorOnlyOverlay.xul b/editor/ui/composer/content/editorOnlyOverlay.xul
I'm not a big fan of this name. Obviously all the good names (editor, composer, compose) are all taken, but I came up with editingOverlay.xul, what do you think?

>             <menupopup id="optionsMenuPopup">
>               <menuitem id="menu_selectAddress"
>                         label="&selectAddressCmd.label;"
>                         accesskey="&selectAddressCmd.accesskey;"
>                         command="cmd_selectAddress"/>
>-              <menuitem id="menu_checkspelling"
>-                        label="&checkSpellingCmd2.label;"
>-                        accesskey="&checkSpellingCmd2.accesskey;"
>-                        key="key_checkspelling"
>-                        command="cmd_spelling"/>
>-              <menuitem id="menu_inlineSpellCheck"
>-                        type="checkbox"
>-                        label="&enableInlineSpellChecker.label;"
>-                        accesskey="&enableInlineSpellChecker.accesskey;"
>-                        oncommand="EnableInlineSpellCheck(!InlineSpellCheckerUI.enabled);"/>
Not sure Mnyromyr will be happy with you moving these menuitems, so forwarding the review.
Attachment #554762 - Flags: review?(neil) → review?(mnyromyr)
Attached patch Renamed share edit menu (obsolete) — Splinter Review
Changes since last version:
* Renamed overlay as suggested
* Various minor tweaks

Note: this does depend on having patches from both bug 21432 and bug 676991 installed.
Attachment #554762 - Attachment is obsolete: true
Attachment #554762 - Flags: review?(mnyromyr)
Attachment #562045 - Flags: review?(mnyromyr)
Depends on: 21432, 676991
Blocks: 688765
Comment on attachment 562045 [details] [diff] [review]
Renamed share edit menu

Just one nit:
>+    <menuitem id="menu_inlinespellcheck"

TB and MailNews both use menu_inlineSpellCheck, only editor uses menu_inlinespellcheck. Thus, I'd rather like to see menu_inlinespellcheck dumped.
r=me with that.
Attachment #562045 - Flags: review?(mnyromyr) → review+
Changes since last version:
* As suggested menu_inlinespellcheck changed to menu_inlineSpellCheck

Carrying forward r=me
Attachment #562045 - Attachment is obsolete: true
Attachment #562235 - Flags: review+
Comment on attachment 562235 [details] [diff] [review]
Uppercase SpellCheck shared edit menu

Requesting TB review on changes to editorOverlay.dtd
Attachment #562235 - Flags: review?(mbanner)
Fix a couple of changes required in editor.js
Attachment #562235 - Attachment is obsolete: true
Attachment #562235 - Flags: review?(mbanner)
Attachment #562288 - Flags: review+
Comment on attachment 562288 [details] [diff] [review]
Complete uppercase spellcheck [Checked in: Comment 10]

Request review for shared changes.
Attachment #562288 - Flags: review?(mbanner)
Blocks: 690145
Blocks: 694027
Blocks: 695842
Comment on attachment 562288 [details] [diff] [review]
Complete uppercase spellcheck [Checked in: Comment 10]

Review of attachment 562288 [details] [diff] [review]:
-----------------------------------------------------------------

::: editor/ui/locales/jar.mn
@@ +3,5 @@
>  @AB_CD@.jar:
>  % locale editor @AB_CD@ %locale/@AB_CD@/editor/
>    locale/@AB_CD@/editor/editorSmileyOverlay.dtd              (%chrome/composer/editorSmileyOverlay.dtd)
>    locale/@AB_CD@/editor/editorOverlay.dtd                    (%chrome/composer/editorOverlay.dtd)
> +  locale/@AB_CD@/editor/editingOverlay.dtd                   (%chrome/composer/editingOverlay.dtd)

I'd really like this to be in a suite-specific directory, or for us to be working towards moving the non-composer strings into a different directory or something, so that Thunderbird localisers don't have to localise extra strings if they aren't doing SeaMonkey as well.

I'm not going to block landing on this, but just want to note it.
Attachment #562288 - Flags: review?(mbanner) → review+
Blocks: 698618
(In reply to Mark Banner (:standard8) from comment #8)
> Comment on attachment 562288 [details] [diff] [review] [diff] [details] [review]
> Complete uppercase spellcheck

> I'd really like this to be in a suite-specific directory, or for us to be
> working towards moving the non-composer strings into a different directory
> or something, so that Thunderbird localisers don't have to localise extra
> strings if they aren't doing SeaMonkey as well.
> 
> I'm not going to block landing on this, but just want to note it.

Spun off into bug 698618 to investigate.
Comment on attachment 562288 [details] [diff] [review]
Complete uppercase spellcheck [Checked in: Comment 10]

http://hg.mozilla.org/comm-central/rev/cd6be6781ce7
Attachment #562288 - Attachment description: Complete uppercase spellcheck → Complete uppercase spellcheck [Checked in: Comment 10]
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.7
FYI this subtle change at http://hg.mozilla.org/comm-central/rev/cd6be6781ce7#l7.27 is likely to be missed by localizers, because the string id hasn't been changed:

-<!ENTITY checkSpellingCmd2.label "Check Spelling">
+<!ENTITY checkSpellingCmd2.label "Check Spelling…">
In cases like this the patch submitter is supposed to send a heads-up message to the L10n newsgroup.
You need to log in before you can comment on or make changes to this bug.