657161 - Make use of contentAreaContext in Composer

Status: RESOLVED FIXED

(SeaMonkey :: Composer, enhancement)

Description

[Ian Neal]

Attachment: Composer context menu patch v1.0

Currently, even though we overlay editor.xul with contentAreaContextOverlay.xul we don't actually make use of the context menu from it. If we did, we could remove some code duplication.
This patch:
* Adapts EditorContextMenuOverlay.xul to overlay contentAreaContextOverlay.xul
* Rewrites EditorContextMenu.js to extend nsContextMenu for Composer window.
* Removes editorContentContext and editorSourceContext context menus.
* Adds IsInPreviewMode() function.
* Adds unique entities for cut, copy and paste toolbarbuttons.
* Remove unused entities from editorOverlay.dtd as they're now provided via contentAreaContextOverlay.

Comment 1

[[e-mail]]

Comment on attachment 532464 [details] [diff] [review]
Composer context menu patch v1.0

>+      window.updateCommands("mode_switch");
Why do we need this? I can't work out what (if anything) it's replacing...

>+  this.showItem("menu_pasteNoFormatting_cm", aShow);
[Hmm, both editor and compose have paste without formatting? Might be worth a followup to move it to contentAreaContextOverlay. Possibly enabling it for all editable regions. Very possibly including paste as quotation, because otherwise the compose overlay will get lonely!]

>   var objectName = InitObjectPropertiesMenuitem("objectProperties_cm");
...
>   InitRemoveStylesMenuitems("removeStylesMenuitem_cm", "removeLinksMenuitem_cm", "removeNamedAnchorsMenuitem_cm");
...
>   InitJoinCellMenuitem("joinTableCells_cm");
Might be possible to avoid these if we're not showing them?

>+  var inCell = IsInTableCell() && aShow;
Nit: aShow && IsInTableCell()

Comment 2

[[e-mail]]

(In reply to comment #1)
>(From update of attachment 532464 [details] [diff] [review])
>>+      window.updateCommands("mode_switch");
>Why do we need this? I can't work out what (if anything) it's replacing...
From IRC, it's to fix a regression from bug 451960. But it's the wrong fix.

Comment 3

[Ian Neal]

Attachment: Composer context menu patch v1.1

Changes since v1.0:
* Moved code only needed when aShow is true into its own if statement.
* Adjusted InitObjectPropertiesMenuitem function so that img special case work does not have to be duplicated.
* Fixed objectProperties regression, InitObjectPropertiesMenuitem now manipulates cmd_objectProperties.
* Separators are hidden/shown as required.

Comment 4

[[e-mail]]

Comment on attachment 532537 [details] [diff] [review]
Composer context menu patch v1.1

Just a skim; haven't tried this out yet.

>+  // Show "Create Link" if not in a link and not in source mode or sidebar.
>+  this.showItem("createLink_cm", !isInLink && aShow);
>+
>+  // Show "Edit link in new Composer" if in a link and
>+  // not in source mode or sidebar.
>+  this.showItem("editLink_cm", isInLink && aShow);
Nit: I'd probably write aShow first on these too for consistency.

>   // Update enable states for all table commands
>   goUpdateTableMenuItems(document.getElementById("composerTableMenuItems"));
[I wonder whether we can optimise this at all.]

>+  <menuitem id="objectProperties"
>+            command="cmd_objectProperties"/>
Won't this fit on one line? ;-)

Thunderbird has a fork of editorOverlay.xul, will these changes break it? In particular, I think they might need this specific change ported; I believe they deleted the other buttons from their copy.

Comment 5

[Ian Neal]

Attachment: Composer context menu patch v1.2 [Checked in: Comment 8]

Changes since v1.1:
* Changed position of aShow in expression for showItem on editLink_cm and createLink_cm.
* Moved goUpdateTableMenuItems into aShow if statement.
* Corrected objectProperties menuitem to be on a single line.
* Ported objectProperties change in editor's editorOverlay to TB's editorOverlay

Comment 6

[Ian Neal]

Comment on attachment 532926 [details] [diff] [review]
Composer context menu patch v1.2 [Checked in: Comment 8]

Requesting review for changes that impact on TB.

Comment 7

[Ian Neal]

Comment on attachment 532926 [details] [diff] [review]
Composer context menu patch v1.2 [Checked in: Comment 8]

Whoever can review the TB related bits first:
editor/ui/composer/content/editor.js
editor/ui/composer/content/editorUtilities.js
editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd
mail/components/compose/content/editorOverlay.xul
I'd like to try and get this into comm-aurora before the merge.

Comment 8

[Ian Neal]

Comment on attachment 532926 [details] [diff] [review]
Composer context menu patch v1.2 [Checked in: Comment 8]

http://hg.mozilla.org/comm-central/rev/04c68787486d
Only difference between this patch and what was checked in was a similar workaround as to in Bug 674246 for the issue in bug 672258