User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0

Steps to reproduce:

Open a new tab.

Actual results:

Since thunderbird version 91 the tab close in the tab context menu does not work anymore. The close button and the key combination ctrl+w can be executed successfully.
Tested also with a new profile and the beta versions 92 and 93.

Expected results:

Right click should close the tab.

Henry, can you take this as you worked in this area?

Ok. This is because the event handler uses triggerNode https://searchfox.org/comm-central/rev/bbe4fa02ba9884831453c336de90fb840a7185e6/mail/base/content/messenger.xhtml#403 and it assumes this is a tab, when it is actually one of the tab's descendants. The other menu items that use triggerNode should be similarly broken.

Not sure what caused this change (I ruled out my own changes to the tab since this effects 91, which is before my changes).

Pushed by alessandro@thunderbird.net:
https://hg.mozilla.org/comm-central/rev/d6792227d11c
Fix the tabContextMenu event handlers. r=mkmelin

Just thought I should add a test for this.

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/6330fa70521f
Add tests for closing tabs using the context menu. r=mkmelin

Comment on attachment 9244771 [details]
Bug 1734413 - Fix the tabContextMenu event handlers. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: Tab context menu does not work (other than "Recently Closed Tabs")
Testing completed (on c-c, etc.): There was an existing test for "Recently Closed Tabs", which still passes. Added tests for "Close Tab" and "Close Other Tabs". No test for "Move to New Window".
Risk to taking this patch (and alternatives if risky): Medium risk. This re-factored how the context menu was handled, but its a small change overall and doesn't interact much with the rest of the code.

Comment on attachment 9245304 [details]
Bug 1734413 - Add tests for closing tabs using the context menu. r=mkmelin

[Approval Request Comment]
Uplift at the same time as the other patch as in comment 8 above.
This is an additional test that covers closing tabs with the context menu.

Comment on attachment 9244771 [details]
Bug 1734413 - Fix the tabContextMenu event handlers. r=mkmelin

[Triage Comment]
Approved for beta - because having tests mitigates the risk

Comment on attachment 9245304 [details]
Bug 1734413 - Add tests for closing tabs using the context menu. r=mkmelin

[Triage Comment]
Approved for beta

Thunderbird 94.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/261fca7ae2fd
Thunderbird 94.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/f84a1e7e8c45

Verified using 94.0b3 on Windows 10.

Comment on attachment 9244771 [details]
Bug 1734413 - Fix the tabContextMenu event handlers. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: Tab context menu does not work (other than "Recently Closed Tabs")
Testing completed (on c-c, etc.): There was an existing test for "Recently Closed Tabs", which still passes. Added tests for "Close Tab" and "Close Other Tabs". No test for "Move to New Window".
Risk to taking this patch (and alternatives if risky): Medium risk. This re-factored how the context menu was handled, but its a small change overall and doesn't interact much with the rest of the code.

Comment on attachment 9245304 [details]
Bug 1734413 - Add tests for closing tabs using the context menu. r=mkmelin

[Approval Request Comment]
Uplift at the same time as the other patch as in comment 14 above.
This is an additional test that covers closing tabs with the context menu.

Dear Maintainer,

the bug is unfortunately still present in the version stable 91.2.1.
In version 94.0b4, closing the tab from the context menu of the respective open tab works ok.

regards
MB

(In reply to bugs@unity-mail.de from comment #16)

Dear Maintainer,

the bug is unfortunately still present in the version stable 91.2.1.

Correct. You can tell by the fact that so far it is only fixed in beta - see comment 12.

When it get fixed in version 91 you will see something like
https://hg.mozilla.org/releases/comm-esr91/rev/261fca7ae2fd

oh, sorry for my impatience...

Comment on attachment 9244771 [details]
Bug 1734413 - Fix the tabContextMenu event handlers. r=mkmelin

[Triage Comment]
Approved for esr91

Thunderbird 91.3.0:
https://hg.mozilla.org/releases/comm-esr91/rev/8bddf18179bd

(second patch with tests intentionally not uplifted)

(In reply to Rob Lemley [:rjl] from comment #20)

Thunderbird 91.3.0:
https://hg.mozilla.org/releases/comm-esr91/rev/8bddf18179bd

(second patch with tests intentionally not uplifted)

So there is more work needed here?

(In reply to Wayne Mery (:wsmwk) from comment #21)

(In reply to Rob Lemley [:rjl] from comment #20)

Thunderbird 91.3.0:
https://hg.mozilla.org/releases/comm-esr91/rev/8bddf18179bd

(second patch with tests intentionally not uplifted)

So there is more work needed here?

Do you mean to needinfo Rob? I'm not sure why the test wasn't uplifted to 91.

The test patch did not apply cleanly on esr91, as it depends on changes from bug 1731497. The patch from bug 1731497 also does not apply cleanly. That's as far as I got with it.

(In reply to Rob Lemley [:rjl] from comment #23)

The test patch did not apply cleanly on esr91, as it depends on changes from bug 1731497. The patch from bug 1731497 also does not apply cleanly. That's as far as I got with it.

Hmm, but bug 1731497 is flagged as not affecting esr91

(In reply to Wayne Mery (:wsmwk) from comment #24)

Hmm, but bug 1731497 is flagged as not affecting esr91

It probably doesn't. From a code perspective though, the changes in the second patch (test changes) in this bug were made atop the changes made in bug 1731497. What's needed is an esr91 port of 6330fa70521f.

Comment on attachment 9245304 [details]
Bug 1734413 - Add tests for closing tabs using the context menu. r=mkmelin

(In reply to Rob Lemley [:rjl] from comment #25)

(In reply to Wayne Mery (:wsmwk) from comment #24)

Hmm, but bug 1731497 is flagged as not affecting esr91

It probably doesn't. From a code perspective though, the changes in the second patch (test changes) in this bug were made atop the changes made in bug 1731497. What's needed is an esr91 port of 6330fa70521f.

K

[Triage Comment]
Approved for esr91