Closed Bug 657161 Opened 12 years ago Closed 12 years ago
Make use of content
Area Context in Composer
(SeaMonkey :: Composer, enhancement)
(Reporter: iannbugzilla, Assigned: iannbugzilla)
(Blocks 1 open bug)
(1 file, 2 obsolete files)
32.92 KB, patch
|Details | Diff | Splinter Review|
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.
12 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()
12 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.
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
12 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.
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)
12 years ago
Attachment #532926 - Flags: review?(neil) → review+
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 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.
12 years ago
Attachment #532926 - Flags: review+
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]
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.5
You need to log in before you can comment on or make changes to this bug.