1716976 - Context menu API: make compose body an editable context

Status: RESOLVED FIXED

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

Description

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

Follow up from bug 1656506 and bug 1670832. The compose body needs to be made an “editable” context.

See bug 1656506 comment #18:

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.

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

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

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

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>.

Comment 1

[John Bieling (:TbSync)]

I have seen this. Will work on it as soon as time permits.

Comment 2

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

I know you are busy, but this has broken 50% of the functionality of my Unicodify add-on, so is there a chance that it could be fixed during the TB 91 cycle. Thanks in advance.

Comment 3

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

Setting status as New per bug 1656506 comment #20.

Comment 4

[John Bieling (:TbSync)]

@Teal: Do you know how to pull a build from the try server? Could you check if this works for you as intended here:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=09debe397d29cd7d7d59f22677b937e0470a97e2

Comment 5

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

Attachment: image.png

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

@Teal: Do you know how to pull a build from the try server? Could you check if this works for you as intended here

Yes, it works great! Thank you for working on this. 🙏

Just a couple of minor notes. One of my add-ons is now listed twice (see attached screenshot). Also, I would not consider the message body a "field" like the other inputs in the compose window, so I do not think you need to set the fieldId property.

Comment 6

[John Bieling (:TbSync)]

How would you know that the context menu was opened on the composeBody, if fieldId was not set?

Comment 7

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

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

How would you know that the context menu was opened on the composeBody, if fieldId was not set?

My add-ons have always assumed that it the compose body by default when using the editable context, unless the fieldId is set to tell them otherwise. My understanding is that this fieldId property was added to TB because those other inputs in the compose window may require special handling by the developer since they cannot be accessed with a standard compose script (bug 1641575). For example, when the fieldId property is set, our Unicodify add-on copies the text to the clipboard as asks the user to manually paste it instead of doing that automatically, which it will now be able to do in the compose body after this bug is fixed (see above screenshot).

Comment 8

[John Bieling (:TbSync)]

I found and fixed the reason for the double entries. But while thinking about the fieldId, I started to ponder if "editable" (alone) is the wrong context altogether. That context is aiming at in-content elements like input fields. The compose window is not really in-content.

Therefore, I would like to introduce a dedicated compose_body context. I can still make it editable, so all other add-ons which add useful generic menu entries to editable elements can still do it, but I assume the compose body is far more special and menu entries designed for it, may not be required on all editable elements. Your thoughts?

Comment 9

[John Bieling (:TbSync)]

Attachment: Bug 1716976 - Add compose_body context and also add composer to editable context. r=darktrojan

Comment 10

[John Bieling (:TbSync)]

try: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=1bdf992882d30c48c450f318e12389a115053b60

Comment 11

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

Attachment: image.png

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

Therefore, I would like to introduce a dedicated compose_body context. I can still make it editable, so all other add-ons which add useful generic menu entries to editable elements can still do it, but I assume the compose body is far more special and menu entries designed for it, may not be required on all editable elements. Your thoughts?

Yes, a new compose_body context sounds like a good addition for people using compose scripts and who only want access to the compose body. However, my primary interest is in making the compose body an editable context to restore compatibility with Firefox. As discussed in bug 1656506, the compose body in Thunderbird behaves the same as a webpage in Firefox with Document.designMode set to on (I believe this is also how it is implemented) and in this case Firefox would enable the editable context for the entire page as expected. As long as your patch fixes this in TB, I (and other developers) will be very happy. 😀

I tried your latest try build in comment 10 and the editable context is no longer firing in the compose body (see attached screenshot).

Comment 12

[John Bieling (:TbSync)]

I tried your latest try build in comment 10 and the editable context is no longer firing in the compose body (see attached screenshot).

Huh, it should. What OS? Can you add a breakpoint in MsgComposeCommands.js in line 1950 and tell me the values of

• editFlags (line 1943)
• onEditable (line 1946)

Comment 13

[John Bieling (:TbSync)]

Attachment: debugger.jpg

This is what I see: editFlags is 288 and onEditable is true.

Comment 14

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

Attachment: image.png

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

Huh, it should. What OS? Can you add a breakpoint in MsgComposeCommands.js in line 1950 and tell me the values of

• editFlags (line 1943)
• onEditable (line 1946)

I am using Windows 11 version 22H2. I had never debugged TB code, so it took me a minute to figure out how to add that breakpoint... The breakpoint on line 1950 did not work for some reason, but I was able to put one above on line 1944 and then step through the code.

Anyway, I get editFlags is 288 (the same as you), but onEditable is false. Hope that helps.

Comment 15

[John Bieling (:TbSync)]

Thank you for your effort!

To know why onEditable comes back as false, could you hover over the values in line 1947 (EDITABLE and CONTENTEDITABLE) and tell me what they are?

Comment 16

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

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

To know why onEditable comes back as false, could you hover over the values in line 1947 (EDITABLE and CONTENTEDITABLE) and tell me what they are?

The values did not make sense, as (288 & (1 | 32)) !== 0 should be true (32 !== 0), but it was false. Long story short, I installed the new try build over the old one (I figured this is what I was supposed to do), but I think for some reason TB had cached and was still using the old version of that file, even though the debugger was showing the current version. Anyway, I installed the latest version of TB Daily and then reinstalled the new try build and it is now all working as expected. Sorry for the false alarm...

Comment 17

[John Bieling (:TbSync)]

Don't be sorry at all. Thanks for your time to verify the patch. That is very much appreciated.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/32a101a15f25
Add compose_body context and also add composer to editable context. r=darktrojan