1656506 - Context menu API: support editable fields in the compose window

Status: RESOLVED FIXED

(Thunderbird :: Add-Ons: Extensions API, enhancement)

Description

[Teal Dulcet [:tdulcet] (TB Council)]

The context menus API currently does not support any of the editable fields in the compose window, including the message body and subject, even if you set the contexts parameter to ["editable"] or ["all"]. I am trying to create a simple extension where users can highlight editable text to quickly change the case or change the characters to a different Unicode font.

Comment 1

[John Bieling]

Confirmed this is true. Also the onShown event does not fire for any of the menus in the composer window.

Comment 2

[John Bieling]

onShown returns sensitive information if host permission is given (compose permission). I need to update the doc.

Comment 3

[John Bieling]

To have this written down somewhere (correct me if I am wrong):

The menu API has two hooks into the context menus:

1. An observer for on-build-contextmenu.

   • triggered directly, for example in openEditorContextMenu(event) (used in a popupshowing event of a menupop) or
   • triggered via new nsContextMenu in for example fillMailContextMenu(event) (used in a popupshowing event of a menupopup)

   The observer gets the preprocessed data which is needed for the info object for the onShown API event.

2. Manually attaching the handleEvent method of the menuTracker of the menu API to a specified set of menus, for example folderPaneContext.

So when adding a new context, one can either update its exiting onpopupshowing event method to prepare the info object and notify the observer, or add its menu(popup) to the list of manual menus and prepare the info object in the handleEvent method of the menuTracker.

However, a few elements do not have a context menu attached. Their contextmenu event is collected by
https://hg.mozilla.org/mozilla-central/file/tip/toolkit/content/editMenuOverlay.js#l96

The nasty thing here is, that the context-menu is added to the DOM on the fly on demand. So the onWindowOpen method of the menu API menuTracker does not see it.
https://hg.mozilla.org/comm-central/file/tip/mail/components/extensions/parent/ext-menus.js#l990

Adding a eventListener to window does not help, as triggerNode is not set and we have no idea what element caused the ˋˋpopupshowingˋˋ event.

Comment 4

[John Bieling]

Attachment: bug1656506_getting_an_idea.patch

I mainly would like to know if this is going in the right direction.

Lets start with the msgSubject field. Its menupopup is anonymous, so I cannot use its ID, instead I setup a global listener for onpopupshowing and catch it by looking at the trigger. I think one could (or even has to) remove the listeners for the individual menupopups now (including the menuIds array).

The subject field is not really content, but returning the same tab which is returned by a composeAction button seems plausible. Providing a PageURL seems not useful.

Now to the address fields. The global onpopupshowing listener also triggeres on all textbox-contextmenu popups which is apparently used if elements (and any of their parents) do not have a contextmenu specified. This is the case for a lot of input fields:

• the address fields in the composer,
• the search fields in the main-3-pane,
• the search field in the address book,
• the search field in the chat
• any input field in the preference tab
• ...

and also on some fields which are already part of the editable context:

• any input field of loaded content pages (for example ATN)
• any input field in the add-on manager (which is probably due to proxy-context-menu used in https://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/content/aboutaddons.html#66 which is needed for the inline options)

I guess we cannot and should not add all these fields to the editable context. I do think the searchfields (in chat, addressbook and mail-3-pane) and the address fields should be part of the editable context, for the sake of continuity. To get that done, I need to be able to identify them and the best option for that is to get bug 1693577 landed. But is it the right approach in general?

In this patch I also added the addresspills themselves to the editable context, just to see if it is working, but I think - if we expose it - it needs its own context. Should we?

Comment 5

[Geoff Lankow (:darktrojan)]

Comment on attachment 9204172 [details] [diff] [review]
bug1656506_getting_an_idea.patch

Review of attachment 9204172 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, the dynamic addition of these menus is unfortunate for this case, so I think adding an overall listener and ignoring the events we want is probably the cleanest option.

::: mail/components/extensions/parent/ext-menus.js
@@ +1006,5 @@
>    },
>  
>    handleEvent(event) {
>      const menu = event.target;
> +    console.log(menu.id);

It looks like the id is available when it's textbox-contextmenu, so can make a case block for it.

The subject context menu looks to be part of the moz-input-box CE, so you could add an identifier to it in the compose window initialisation code.

@@ +1047,5 @@
>        }
> +      default: {
> +        // Unspecified onpopupshowing from the global listener.
> +        let triggerNode = event.target.triggerNode;
> +        let triggerID = triggerNode?.id;

Try document.activeElement here. The small amount of looking I've done suggests it's usable, although it may be a descendant of the element you want to look at.

Comment 6

[Geoff Lankow (:darktrojan)]

Another problem I've just thought of is that we don't really have a way to interact with the actual fields in question. For example in the subject box, I might be able to find out what text is selected when the menu opens but I can't replace just that selected text with something else. (Also I might want to do something like that from a compose action or elsewhere.) Not sure that's really in scope here but it's something to think about.

Comment 7

[Teal Dulcet [:tdulcet] (TB Council)]

(In reply to Geoff Lankow (:darktrojan) from comment #6)

Another problem I've just thought of is that we don't really have a way to interact with the actual fields in question. For example in the subject box, I might be able to find out what text is selected when the menu opens but I can't replace just that selected text with something else.

I specifically requested this feature in bug 1641575, but unfortunately you marked it as WONTFIX... I wanted to be able to make changes to the subject in the same manner as the message body, including via the context menu.

Comment 8

[Geoff Lankow (:darktrojan)]

Sort of. The subject field is not a document and therefore can't run scripts, so what we really want is some functions for interacting with the selection.

Comment 9

[Teal Dulcet [:tdulcet] (TB Council)]

The message body behaves like a ContentEditable Div and the subject like an <input type="text"> element. If the subject was added to the same document for compose scripts as the message body, even if in a separate frame, it would allow extensions to make changes to them both in the same manner.

The same code could be used for both in extensions, regardless of whether the user opened the context menu in the message body or the subject. It would also not require developers to make any changes when porting existing content scripts in Firefox extensions to compose scripts for Thunderbird.

Comment 10

[John Bieling]

Attachment: Bug 1656506 - Context menu API: support editable fields in the compose window. r=darktrojan

Comment 11

[John Bieling]

(In reply to Geoff Lankow (:darktrojan) [away until 12/4] from comment #6)

Another problem I've just thought of is that we don't really have a way to interact with the actual fields in question. For example in the subject box, I might be able to find out what text is selected when the menu opens but I can't replace just that selected text with something else. (Also I might want to do something like that from a compose action or elsewhere.) Not sure that's really in scope here but it's something to think about.

After bug 1693577 landed I could continue to work on this. The updated patch adds the subject and the address input fields to the editable context and also returns selections, but it does not identify the fields. The menus API has getTargetElement() to get the entire element, but we cannot use that, as those elements are not content.

Should we add a subject_field context (and a to/cc/bcc_field context), or should we include an additional member in the contextData, similar to getTargetElement(), but which just returns an object with a getter/setter for the fields value and a getter for its id? The fields are already accessible via set/getComposeDetails(), so another setter/getter seems too much. But some sort of identification which can be used in menus.onShown to dynamically add the menu is probably still needed. Whatever we do, it should be a solution which can also be used in other Thunderbird UI fields, which are not content but where WebExtension might need access.

Edit: Fixed wrong bug reference.

Comment 12

[Geoff Lankow (:darktrojan)]

(In reply to John Bieling (:TbSync) from comment #11)

The menus API has getTargetElement() to get the entire element, but we cannot use that, as those elements are not content.

Incidentally, this isn't in our documentation because it's in menus_child.json not menus.json. That should be fixed.

(In reply to John Bieling (:TbSync) from comment #11)

Should we add a subject_field context (and a to/cc/bcc_field context), or should we include an additional member in the contextData, similar to getTargetElement(), but which just returns an object with a getter/setter for the fields value and a getter for its id? The fields are already accessible via set/getComposeDetails(), so another setter/getter seems too much. But some sort of identification which can be used in menus.onShown to dynamically add the menu is probably still needed. Whatever we do, it should be a solution which can also be used in other Thunderbird UI fields, which are not content but where WebExtension might need access.

Should we just add an identifying string to the info object? We're going to want some kind of interface for dealing with input fields which will need identifiers anyway. Something like fieldId: "composeTo" (compose prefix to avoid confusion with the message header)?

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/26ee3b18bb67
Context menu API: support editable fields in the compose window. r=darktrojan

Comment 14

[Teal Dulcet [:tdulcet] (TB Council)]

Attachment: image.png

This and bug 1670832 should not have been closed, as the context menu still does not support the message body when you set the contexts parameter to ["editable"]. browser.menus.onShown does fire (see attached screenshot), but none of the items show up in the context menu. It looks like the message body was not marked as editable.

Comment 15

[John Bieling]

Try "page" context for the compose body.

Comment 16

[Teal Dulcet [:tdulcet] (TB Council)]

(In reply to John Bieling (:TbSync) from comment #15)

Try "page" context for the compose body.

That unfortunately does not work when text is selected. The “selection” context works, but it also fires for non-editable text too, like displayed messages. Should I open a new bug just for supporting the compose body or should I reopen this bug?

Comment 17

[John Bieling]

The "page" context does not fire for selected text, but only the "selection" context? Hm, this does seem wrong. IIRC correctly, other contexts always fire and the "selection" context fires additionally. Could you verify if this happens in Firefox as well?

Comment 18

[Teal Dulcet [:tdulcet] (TB Council)]

(In reply to John Bieling (:TbSync) from comment #17)

The "page" context does not fire for selected text, but only the "selection" context?

Yes, see the screenshot in comment #14. In editable fields like the compose body, both the “selection” and “editable” contexts should fire for selected text.

Hm, this does seem wrong. IIRC correctly, other contexts always fire and the "selection" context fires additionally. Could you verify if this happens in Firefox as well?

In Firefox in a ContentEditable Div, the “page” context does not fire, only the “editable” context and “selection” if text is selected. According to MDN, the page context “applies when the user context-clicks in the page, but none of the other page contexts apply”.

As I said before, I believe the issue is that the compose body was not marked as editable, so the “editable” context is not firing. In the Compose scripts API, the isContentEditable property is also not set for the compose body, which may be another synonym of the same issue. The subject and the rest of the editable fields in the compose window now work fine.

Comment 19

[John Bieling]

Geoff, should the compose body be an "editable" context? Comment 18 has all the important details.

Comment 20

[Geoff Lankow (:darktrojan)]

I guess so. It's probably not that way already because it's a normal document in an <editor> rather than an editable document in a <browser>. (Side thought: I don't really know why <editor> is still a thing when documents can be edited in a <browser>. There must be some reason.)