1875802 - Implement the new message context menu structure

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, task)

Description

[Richard Marti (:Paenglab)]

In https://thunderbird.topicbox.com/groups/ux/T3d84faa372bf41a8/message-context-menu-updates is a mock-up how the new context menu could look.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: Bug 1875802 - Implement the new message context menu structure. r=#thunderbird-front-end-reviewers

Comment 2

[Elizabeth Mitchell]

Attachment: Updated button styling

Updated button styling

Comment 3

[Martin Giger [:freaktechnik]]

Attachment: Bug 1875802 - Fix browser_threads for contextmenu submenus. r=#thunderbird-front-end-reviewers

Comment 4

[Emilio Cobos Álvarez (:emilio)]

So I can reproduce the failure here, but only on Wayland, can you confirm that's what you see?

What I see is that we fire a menu command from here.

What seems to happen is that the inner menu is getting forcefully closed as a result of a bounds change in here, triggered from the .scrollTop in here:

call DumpJSStack()
0 _ensureVisibleRowsAreDisplayed() ["chrome://messenger/content/tree-view.mjs":1014:9]
    this = [object HTMLElement]
1 invalidateRange(startIndex = "0", endIndex = "0") ["chrome://messenger/content/tree-view.mjs":741:10]
    this = [object HTMLElement]
2 invalidateRange(startIndex = "0", endIndex = "0") ["chrome://messenger/content/about3Pane.js":4506:20]
    this = [object Object]
3 _navigate(navigationType = "5") ["chrome://messenger/content/mailCommon.js":864:27]
    this = [object Object]
4 anonymous("[object XULCommandEvent]") ["chrome://messenger/content/about3Pane.js":6445:23]
5 doCommand(command = ""cmd_killThread"", args = "[object XULCommandEvent]") ["chrome://messenger/content/mailCommon.js":734:38]
    this = [object Object]
6 onMailContextMenuCommand(event = "[object XULCommandEvent]") ["chrome://messenger/content/mailContext.js":496:25]
    this = [object Object]
7 onCommand(event = "[object XULCommandEvent]") ["chrome://messenger/content/mailContext.js":157:10]
    this = [object Object]
8 handleEvent(event = "[object XULCommandEvent]") ["chrome://messenger/content/mailContext.js":139:14]
    this = [object Object]

And then when we go and try to close the chain here and we don't find it (because the inner menu is already closed) and end up in a wrong state.

So it's a bug in the popup manager of sorts, but you could also not force the closing of the popup from the command event... Any reason why _ensureVisibleRowsAreDisplayed needs to synchronously flush layout? It seems you just want to make sure it happens before the next frame (so making sure you schedule it in requestAnimationFrame() should work).

Also let me know if you can repro the failure on Windows, macOS, or Linux in X11 (or with MOZ_ENABLE_WAYLAND=0), because otherwise that's not what I've been debugging.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

The other thing to check is why closing the popup changes the layout of the popup, seems like that shouldn't happen.

Comment 6

[Martin Giger [:freaktechnik]]

Okay, that explanation makes sense with the change and everything. I did run the test on Wayland too, yes. Alex said he saw the same, not sure what he was using.

I'll try it without wayland tomorrow and also investigate if we can move _ensureVisibleRowsAreDisplayed into an animation frame or use non-flushing sizings.

Thanks for looking into it!

Comment 7

[Martin Giger [:freaktechnik]]

The test passes in an X11 build just fine indeed.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/57cadfa65d38
Implement the new message context menu structure. r=aleca,freaktechnik
https://hg.mozilla.org/comm-central/rev/66fac7c74dda
Fix browser_threads for contextmenu submenus. r=aleca

Comment 9

[Magnus Melin [:mkmelin]]

comm/mail/base/test/browser/browser_browserContext.js is failing on mac. I'd assume from one of these patches

Comment 10

[Richard Marti (:Paenglab)]

Martin, you know this tests better. Could you look at the failures?

Comment 11

[Martin Giger [:freaktechnik]]

Oh, I think that's pretty simple and comes from the pref value change, we need to update the test expectation to also expect the non-native context menu layout on mac. I'll provide a patch in a regression bug.

Comment 12

[Alfred Peters [:infofrommozilla]]

I missed the "Cancel Messages" item in the context menu - but found it back under "Forward and Redirect".
I suspect it was caused by this bug.
Was that intentional or an accident?
In my opinion, it doesn't really fit in there.

Comment 13

[Richard Marti (:Paenglab)]

This item is very seldom in Newsgroups shown and probably moved there by accident. Best is to file a new bug for this. Then it can be decided where the menuitem can be placed.