655529 - Make use of contentAreaContext in message compose window

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Composition, enhancement)

Description

[Ian Neal]

Attachment: Use contentAreaContext for messenger compose window patch v1.0

Currently, even though we overlay messengercompose.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:
* Adds a getBrowser function to the compose window.
* Removes unneeded openEditorContextMenu function.
* Adds event listener for popupshowing so that updateEditItems is still called.
* Removes msgComposeContext context menu.
* Adds overlay that gives two missing context items for context menu.

Comment 1

[[e-mail]]

I notice that the mailContext has a hybrid approach - it defines its own menuitems but then calls on nsContextMenu to set things up.

Comment 2

[Ian Neal]

Attachment: Use contentAreaContext for messenger compose window alternative patch v1.0

This alternative patch:
* Replaces the onpopupshowing on contentAreaContextMenu, this means a getBrowser function is not needed nor is adding an event listener.

Comment 3

[Ian Neal]

Attachment: Do it the same way as mailContext alternative patch v1.0

This patch:
* Does the context menu the same way as mailContext does in mailWindowOverlay, but I do think this one adds as well as taking away some duplication of code.

Comment 4

[[e-mail]]

Comment on attachment 530935 [details] [diff] [review]
Use contentAreaContext for messenger compose window alternative patch v1.0

>+  gContextMenu =
>+      new nsContextMenu(popup, document.getElementById("content-frame"));
>   updateEditItems();
[As it happens, nsContextMenu already updates the standard edit items.]

>+  <popupset id="contentAreaContextSet">
Nit: don't need this, because you know the id of the popup.

>+    <menupopup id="contentAreaContextMenu"
>+               onpopupshowing="return event.target != this ||
>+                                      openEditorContextMenu(this);">
>+      <menuitem id="context-pasteNoFormatting"
>+                insertafter="context-paste"
>+                command="cmd_pasteNoFormatting"/>
>+      <menuitem insertafter="context-pasteNoFormatting"
>+                label="&pasteQuote.label;"
>+                accesskey="&pasteQuote.accesskey;"
>+                command="cmd_pasteQuote"/>
This almost works, but the sidebar uses the same popup, so the menuitems appear (permanently) on the sidebar content context menu.

Comment 5

[[e-mail]]

Comment on attachment 530875 [details] [diff] [review]
Use contentAreaContext for messenger compose window patch v1.0

Actually I can't decide between either of these patches. I guess it depends on which way turns out to be easier to hide the extra menuitems in the sidebar.

Comment 6

[[e-mail]]

Comment on attachment 531470 [details] [diff] [review]
Do it the same way as mailContext alternative patch v1.0

I see what you mean about the duplication, because we already have a context menu in this window, which means that we don't get the right overlay love.

Comment 7

[[e-mail]]

Comment on attachment 530935 [details] [diff] [review]
Use contentAreaContext for messenger compose window alternative patch v1.0

>+  gContextMenu =
>+      new nsContextMenu(popup, document.getElementById("content-frame"));
>   updateEditItems();
>+  return gContextMenu.shouldDisplay;
This version has the advantage that you could write something like this:

gContextMenu =
    new nsContextMenu(popup, document.getElementById("content-frame"));
if (gContextMenu.shouldDisplay) {
  // handle extra paste items here
}
return gContextMenu.shouldDisplay;

Comment 8

[Ian Neal]

Attachment: Use contentAreaContext for messenger compose window alternative patch v1.1

Changes since v1.0:
* Hide extra menuitems as default and unhide with JS in onpopupshowing.
* Remove unneeded <popupset> tags in overlay.
* Only update extra menuitems as rest are done by nsContextMenu code.

Comment 9

[[e-mail]]

Comment on attachment 532052 [details] [diff] [review]
Use contentAreaContext for messenger compose window alternative patch v1.1

[I don't think this is going to work, because the sidebar also uses context="contentAreaContextMenu" which will call openEditorContextMenu.]

Comment 10

[Ian Neal]

(In reply to comment #9)
> Comment on attachment 532052 [details] [diff] [review] [review]
> Use contentAreaContext for messenger compose window alternative patch v1.1
> 
> [I don't think this is going to work, because the sidebar also uses
> context="contentAreaContextMenu" which will call openEditorContextMenu.]

Okay a custom sidebar tab (added using http://edmullen.net/mozilla/sidebarpreview.php) gives a:
Error: window.getBrowser is not a function
Source file: chrome://navigator/content/navigator.xul
Line: 1

which suggests to me that, probably due to the load order and how overlays work (at the moment), it is not getting overlaid by the new file.

Comment 11

[[e-mail]]

(In reply to comment #10)
> Error: window.getBrowser is not a function
Oh dear. Regression from bug 562339 :-(

Comment 12

[Ian Neal]

Attachment: Use contentAreaContext for messenger compose window alternative patch v1.2

Changes since v1.1:
* Use WhichElementHasFocus() to determine if on message body.
* Take into account fix for non-browser sidebar issue (bug 656886).

Comment 13

[[e-mail]]

Comment on attachment 532199 [details] [diff] [review]
Use contentAreaContext for messenger compose window alternative patch v1.2

>+    var focusedElement = WhichElementHasFocus();
Nit: could optimise this, since you're only interested in == content

>+    var showPasteExtra = focusedElement && focusedElement == content;
Nit: superfluous extra test for focusedElement &&

Comment 14

[Ian Neal]

Attachment: Use contentAreaContext for messenger compose window alternative patch v1.2a [Checked in: Comment 15]

For checkin

Comment 15

[Ian Neal]

Comment on attachment 532331 [details] [diff] [review]
Use contentAreaContext for messenger compose window alternative patch v1.2a [Checked in: Comment 15]

http://hg.mozilla.org/comm-central/rev/ac811b06e1c2