1782486 - After calling CompFields2Recipients() with empty fields, autocomplete fails

Status: RESOLVED FIXED

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

Description

[Mikhail]

Hello guys!

I did not find how to reopen Bug 1773663, so I have to create a new one.

We have investigated the issue and found that looks like function CompFields2Recipients is broken. In Bug 1773663 I mentioned that the problem does not occur if I switch off my addon.

The problem is that my add-on actively uses CompFields2Recipients function.

In ThunderBird 91 if we called CompFields2Recipients with empty fields nothing particular happend.
But in ThunderBird 102 - autocomplete just stops working in that case.

So we have problem with the following types of mails: New, ForwardAsAttachment и ForwardInline. BUT if we make reply, problem does not occur (fields are not empty).

As a walkaround we may use this:

(function (){

Services.wm.addListener({
                    onOpenWindow: function (aWindow) {
                        Services.wm.removeListener(this);
                        this.composeWin = aWindow.docShell.domWindow;
                        this.composeWin.addEventListener("compose-editor-ready", this, true);
                    },
                    handleEvent: function (event) {
                        if (event.type == "compose-editor-ready") {
                            event.currentTarget.removeEventListener(event.type, this, true);
                            let win = event.target.ownerGlobal;
                            let compFields = win.gMsgCompose.compFields;
                           
                            win.CompFields2Recipients(compFields);
                            //it is a cure (if we comment these 4 rows - the problem occurs):
                            try {
                                let row = win.document.getElementById("addressRowTo");
                                let input = row.querySelector(".address-row-input");
                                input.detachController();
                                input.attachController();
                            } catch {}

                        }
                    }
                });
ComposeMessage(Ci.nsIMsgCompType.New, Ci.nsIMsgCompFormat.Default);

})();

Please check our guess about CompFields2Recipients function being broken.
Thank you!

Comment 1

[Thomas D. (:thomas8)]

John, is this add-on territory?

Comment 2

[John Bieling (:TbSync)]

I cannot reproduce this bug. I pasted the following code into Thunderbirds Javascript console and hit enter:

Services.wm.addListener({
    onOpenWindow: function (aWindow) {
        Services.wm.removeListener(this);
        this.composeWin = aWindow.docShell.domWindow;
        this.composeWin.addEventListener("compose-editor-ready", this, true);
    },
    handleEvent: function (event) {
        if (event.type == "compose-editor-ready") {
            console.log("TEST")
            event.currentTarget.removeEventListener(event.type, this, true);
            let win = event.target.ownerGlobal;
            let compFields = win.gMsgCompose.compFields;
            win.CompFields2Recipients(compFields);
        }
    }
});
ComposeMessage(Ci.nsIMsgCompType.New, Ci.nsIMsgCompFormat.Default);

The composer opens, I see "Test" written and autocomplete works. Tested in:

• 105.0a1 (2022-08-04) (64-bit)
• 104.0b2 (32-Bit)
• 102.1.0 (64-Bit)

All on Windows 10.

Are you able to reproduce this in a fresh profile (with a few contacts added to the address book) on any of these versions? If it is not just the code above which is triggering this behaviour for you, it must be something else.

Comment 3

[Mikhail]

Hello!

Thank you for your reply.
We spent sometime for reproducing a bug at fresh profile and looks like we found key factor.
What we did.
We set up fresh profile and found that error did not occur.
But then we add an email at "Reply adress" field in the Account settings and that the problemm immediately back.
I will attach 2 screenshots

Comment 4

[Mikhail]

Attachment: 1.png

Screen shot shows it works

Comment 5

[Mikhail]

Attachment: 2.png

Screen shot shows it does not work

Comment 6

[John Bieling (:TbSync)]

Thanks! With that info, I can recreate it on a fresh profile by passing in a value for any of the allowed values (replyTo, followupTo, ...) but not the currently active one (To):

Services.wm.addListener({
    onOpenWindow: function (aWindow) {
        Services.wm.removeListener(this);
        this.composeWin = aWindow.docShell.domWindow;
        this.composeWin.addEventListener("compose-editor-ready", this, true);
    },
    handleEvent: function (event) {
        if (event.type == "compose-editor-ready") {
            console.log("TEST")
            event.currentTarget.removeEventListener(event.type, this, true);
            let win = event.target.ownerGlobal;
            let compFields = win.gMsgCompose.compFields;
            win.CompFields2Recipients({followupTo: "[e-mail]"});
        }
    }
});
ComposeMessage(Ci.nsIMsgCompType.New, Ci.nsIMsgCompFormat.Default);

This bug is also affecting the WebExtension API. Executing

browser.compose.beginNew({replyTo: "[e-mail]"}) 

will open up a new composer where the autocomplete on the To field is not working either, while it does work after calling

browser.compose.beginNew({to: "[e-mail]"}) 

Setting focus from the To field to the Subject and back fixes it for me. I will try to understand why.

@ alessandro : Do you have an idea, why the autocomplete feature gets disabled for the currently active address input field (To), if CompFields2Recipients() is changing another(!) address input field (replyTo, followupTo, cc, bcc)?

The autocomplete feature is enabled again, if focus is removed and added back.

Comment 7

[John Bieling (:TbSync)]

Attachment: Bug 1782486 - Re-attach autocomplete controller only if it was detached. r=aleca

Comment 8

[John Bieling (:TbSync)]

This is the important stack when calling browser.compose.beginNew({replyTo: "[e-mail]"}) (from top to bottom, for better readability)

• chrome://messenger/content/parent/ext-compose.js#272 :

  setComposeDetails({replyTo: "[e-mail]"})
  

• chrome://messenger/content/parent/ext-compose.js#521 :

  SetComposeDetails({replyTo: "[e-mail]"})
  

• chrome://messenger/content/messengercompose/MsgComposeCommands.js#5567 :

  CompFields2Recipients({replyTo: "[e-mail]"})
  

• chrome://messenger/content/messengercompose/addressingWidgetOverlay.js#114 :

  setAddressRowFromCompField(
      "addressRowReply",
      "[e-mail]",
      true
  )
  

• chrome://messenger/content/messengercompose/addressingWidgetOverlay.js#99 :

  recipientAddPills(
      hbox#replyAddrContainer.input-container.wrap-container.address-container (=input)
      true
  )
  

• chrome://messenger/content/messengercompose/addressingWidgetOverlay.js#856 :

  input.detachController();
  input.attachController()
  

detachController() checks if the current controller belongs to the given input - and since ReplyTo != AddrTo, it does not detach the controller. However, attachController() overwrites the controller nevertheless, which causes the autocomplete to break.

My proposed fix it therefore to attach the controller only if it was detached.

Comment 9

[John Bieling (:TbSync)]

Grrr.

My change to detachController() to return a boolean did not make it into my patch, because it is toolkit code. Will need a different approach.

Comment 10

[John Bieling (:TbSync)]

Checking for !input.mController.input now.

Maybe there is a better way to identify if detach/attach is needed at all?

Comment 11

[John Bieling (:TbSync)]

Successful try run:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=37193adbaa248f103e537d02ed7f4600cadadfa6

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/05446ad14e1a
Re-attach autocomplete controller only if it was detached. r=aleca

Comment 13

[John Bieling (:TbSync)]

Comment on attachment 9288883 [details]
Bug 1782486 - Re-attach autocomplete controller only if it was detached. r=aleca

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
Autocompete in composer does not work under certain conditions

Testing completed (on c-c, etc.):
3 days on Daily

Risk to taking this patch (and alternatives if risky):
I do not see an obvious risk. The code change is minimal.

Comment 14

[Rob Lemley [:rjl]]

Comment on attachment 9288883 [details]
Bug 1782486 - Re-attach autocomplete controller only if it was detached. r=aleca

[Triage Comment]
Approved for beta.

Comment 15

[Rob Lemley [:rjl]]

Thunderbird 104.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/c571c6c50b1d

Comment 16

[John Bieling (:TbSync)]

Comment on attachment 9288883 [details]
Bug 1782486 - Re-attach autocomplete controller only if it was detached. r=aleca

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
Autocompete in composer does not work under certain conditions

Testing completed (on c-c, etc.):
On Beta since 104

Risk to taking this patch (and alternatives if risky):
I do not see an obvious risk. The code change is minimal.

Comment 17

[Wayne Mery (:wsmwk)]

Comment on attachment 9288883 [details]
Bug 1782486 - Re-attach autocomplete controller only if it was detached. r=aleca

[Triage Comment]
Approved for esr102

Comment 18

[Rob Lemley [:rjl]]

Thunderbird 102.3.0:
https://hg.mozilla.org/releases/comm-esr102/rev/b25f458ed03f