Make use of contentAreaContext in Composer

RESOLVED FIXED in seamonkey2.5

Status

SeaMonkey
Composer
--
enhancement
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Ian Neal, Assigned: Ian Neal)

Tracking

(Blocks: 1 bug)

Trunk
seamonkey2.5
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 years ago
Created attachment 532464 [details] [diff] [review]
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.
Attachment #532464 - Flags: review?(neil)

Comment 1

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

Updated

7 years ago
Blocks: 657234

Comment 2

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

Updated

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

Comment 3

7 years ago
Created attachment 532537 [details] [diff] [review]
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.
Attachment #532464 - Attachment is obsolete: true
Attachment #532537 - Flags: review?(neil)

Comment 4

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

Updated

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

Comment 5

7 years ago
Created attachment 532926 [details] [diff] [review]
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
Attachment #532537 - Attachment is obsolete: true
Attachment #532926 - Flags: review?(neil)

Updated

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

Comment 6

7 years ago
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.
Attachment #532926 - Flags: review?(mbanner)
(Assignee)

Updated

7 years ago
Blocks: 660739
(Assignee)

Updated

7 years ago
Blocks: 666259
(Assignee)

Comment 7

7 years ago
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.
Attachment #532926 - Flags: review?(dbienvenu)
Attachment #532926 - Flags: review?(mbanner)
Attachment #532926 - Flags: review?(dbienvenu)
Attachment #532926 - Flags: review+
(Assignee)

Comment 8

7 years ago
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
Attachment #532926 - Attachment description: Composer context menu patch v1.2 → Composer context menu patch v1.2 [Checked in: Comment 8]
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.5
You need to log in before you can comment on or make changes to this bug.