Closed Bug 657161 Opened 10 years ago Closed 10 years ago
Make use of content
Area Context in Composer
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 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()
(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.
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
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.
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: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.5
You need to log in before you can comment on or make changes to this bug.