[macOS] Right-click on a recipient pill fails to select it/clears selection with widget.macos.native-context-menus set to true
Categories
(Thunderbird :: Message Compose Window, defect, P1)
Tracking
(thunderbird_esr91+ fixed, thunderbird93? fixed, thunderbird94+)
People
(Reporter: thomas8, Assigned: aleca)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
(deleted),
text/x-phabricator-request
|
wsmwk
:
approval-comm-beta+
wsmwk
:
approval-comm-esr91+
|
Details |
Thanks to Wayne who found this on TB 93 (on Matrix today).
Maybe regressed. Aleca reports it also affects 91.1.
Makes contextual actions on a single recipient pill with mouse a no-op, which is a pretty pretty significant and annoying everyday loss of functionality. Should fix this asap.
Aleca, could you have a look?
STR
- On Mac Big Sur, right-click on a recipient pill in composition's addressing area
- Check if pill is selected
- Choose
Cut
from context menu
Actual
- On Mac Big Sur / TB 93, right-click no longer selects the pill
- pill not cut; contextual actions all fail (except
Edit Address
)
Expected
- Right-click on a pill must select it first (as it does on Windows)
- pill should be cut away; contextual actions from a plain vanilla right-click on a pill should all work
Assignee | ||
Comment 2•3 years ago
|
||
Confirmed.
This also affects 91.1 on macOS.
Bumping up the severity and priority as this issue is pretty disruptive.
Comment 3•3 years ago
|
||
It also fails for me with 92.0b5. So I don't have a regression range.
Assignee | ||
Comment 6•3 years ago
|
||
- 89b3 works.
- 89b4 has a partial regression (right click doesn't select but if the pill is selected, the right click doesn't provoke a deselection).
- 90b1 doesn't work entirely and the issue remained identical as of now.
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 7•3 years ago
|
||
Found the issue in this code: https://searchfox.org/comm-central/rev/0fc487b31cc488a9fa042a41d16ba6d96a842a18/mail/base/content/widgets/mailWidgets.js#2494-2496
Apparently, native macos context menus trigger a focusout
event.
This doesn't happen on Windows or Linux.
Coming up with a fix.
Assignee | ||
Comment 8•3 years ago
|
||
I'm having issues in figuring out how to solve this. These are my findings:
- The
focusout
listener attached to the entire recipients area doesn't play well with native macos context menus as they steal the focus and causes the pill deselection code to run. - The fact that the event listener is attached to the entire recipient area and we let it bubble up to all its child elements is not ideal as it doesn't leave us much flexibility to handle these edge cases.
Possible solutions/approaches to fix this:
- Force the native macos context menus to not steal the focus if the original target is a pill (not sure if it's doable and if it could affect the keyboard navigation of the context menu items).
- Drop the
focusout
and handleblur
events to the elements we need to listen to (recipients area, input fields, pills) to avoid bubbling and preventing deselection with more fine tuning.
Maybe a smarter approach that is escaping me right now.
Assignee | ||
Comment 9•3 years ago
|
||
Assignee | ||
Comment 10•3 years ago
|
||
I think I fixed all the scenarios with these changes.
I need to test this patch on Linux and Windows to be sure I didn't cause any regression.
If this approach looks correct, I'll add a bunch of tests to cover every scenario for all platforms.
Assignee | ||
Comment 11•3 years ago
|
||
And of course, this patch breaks things on Windows but works perfectly on macOS.
Assignee | ||
Comment 12•3 years ago
|
||
The problem in general is the focusout
event listener which gets triggered with the new macOS native context menus.
The focusout
event is used to properly deselect pills when the focus leaves the recipients area (eg. moves to the contacts sidebar, or any other element).
Another possible solution would be to not use the focusout
event but add a onclick
event to the compose window in order to deselect all selected pills only on specific occasions. This would allow us to ignore the lost focus when opening the context menu.
I'm not sure how hacky something like that might be.
Comment 13•3 years ago
|
||
Using onclick instead seems it could be trouble, but would need some experimentation...
Could we perhaps find a way to ignore focusout if we know it's coming from the context menu?
Reporter | ||
Comment 14•3 years ago
|
||
This might be enough to fix it. Can't test on Mac.
I'm assuming that it's the popup which gets the focus when context menu triggers focusout event on Mac. Otherwise just substitute the context menu ID with whichever element gets focus.
Assignee | ||
Comment 15•3 years ago
|
||
Assignee | ||
Comment 16•3 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #13)
Using onclick instead seems it could be trouble, but would need some experimentation...
I think I have a "not so dirty" patch with this approach.
Could we perhaps find a way to ignore focusout if we know it's coming from the context menu?
I tried, but on macOS the focusout
event triggered by native context menus doesn't come with any unique pointer indicating the origin of it.
Assignee | ||
Comment 17•3 years ago
|
||
Pinging Markus to see if he has any insight on this and if it might be a toolkit issue with the native context menu.
Assignee | ||
Comment 18•3 years ago
|
||
I updated the patch on Phabricator with the onclick
approach.
That fixes everything on macOS, but I need to test things properly on Windows and Linux. I uploaded it for easy cross platform fetching and updating.
Assignee | ||
Updated•3 years ago
|
Comment 19•3 years ago
|
||
Interesting, I wasn't aware of the focus phenomenon with native context menus. Could you file a bug about that? I'm not sure what causes the focus change here, maybe the native window itself gets an "unfocused" notification from macOS.
Assignee | ||
Comment 20•3 years ago
|
||
Updated patch on phabricator now works on all platforms.
Here's an overview of what I did:
- Remove the
focusout
event listener from the entire recipient area (this removes also the issue of detecting if the user is changing to another window) - Handle the selection and deselection of pills through the pill's
onclick
event. - Handle the deselection of single pills through the pill's
onblur
event. - Handle the deselection of multiple pills through the window's
onclick
event, preventing deselection if the click happens on a pill, or an element we want to ignore (right click, menubar, toolbarbutton).
Having a click event attached to the whole window is not optimal, but maybe this could be a good enough solution to fix this problem before 91 gets widely adopted, meanwhile we investigate the cocoa toolkit issue.
Assignee | ||
Comment 21•3 years ago
|
||
Try run launched with newly implemented tests to cover all the scenarios listed above and some context menu actions: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=14e4646ec1486d2df4136028c7fac1a22767450c
Assignee | ||
Comment 22•3 years ago
|
||
And of course synthesizeMouseAtCenter
doesn't work for context menu items on macOS.
Test updated and launched another try run: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=9937e41e993bdc051e7346eeb70aa3fde3776bd5
Comment 23•3 years ago
|
||
tested comment 22 try run on Mac. looks good. Though probable that I didn't cover all the bases.
One unexpected thing to note - I somehow caused the To: field contents to be selected when I did command+A or some variation
Calum, dossy, possible for you to test Mac? https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/BsPTuFvBRr2y4dtXGr_BcA/runs/0/artifacts/public/build/target.dmg
Doug, Walt, possible for you to test Windows? 64bit https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/O1mlezSTQjyaF22LmZ4WOA/runs/0/artifacts/public/build/install/sea/target.installer.exe
try build is based on v94 nightly, so you may want a new profile, unless you are already running nightly
Reporter | ||
Comment 24•3 years ago
|
||
(In reply to Wayne Mery (:wsmwk) from comment #23)
One unexpected thing to note - I somehow caused the To: field contents to be selected when I did command+A or some variation
That's a feature :-)
- First Ctrl/Cmd+A selects all pills in current recipient field (with focus on a pill or empty input; obviously plain text in input will absorb this).
- Another Ctrl/Cmd+A selects all pills in all recipient fields. Very convenient for removing all recipients.
It's also documented:
https://support.mozilla.org/en-US/kb/addressing-email#w_selecting-several-or-all-recipient-items
Comment 25•3 years ago
|
||
(In reply to Thomas D. (:thomas8) from comment #24)
- Another Ctrl/Cmd+A selects all pills in all recipient fields. Very convenient for removing all recipients.
But this also selects the 'Reply-To' pill and you don't really want to remove this one??
Comment 26•3 years ago
|
||
Tested Win 10 64 bit TBird 93.0b1 Right click on pill brings up menu with "Edit Address" etc. Tried To, CC, BCC and Reply-To
Appears to be ok. Well done.
Assignee | ||
Comment 27•3 years ago
|
||
Another try-run to fix one last tiny failing test: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=abc2314b259879fe8e323759af899a1290d870f0
Assignee | ||
Updated•3 years ago
|
Reporter | ||
Comment 28•3 years ago
|
||
(In reply to Carl from comment #25)
(In reply to Thomas D. (:thomas8) from comment #24)
- Another Ctrl/Cmd+A selects all pills in all recipient fields. Very convenient for removing all recipients.
But this also selects the 'Reply-To' pill and you don't really want to remove this one??
Interesting point, Carl, but we can't discuss that here. You could file an RFE. There are pros and cons to excepting Reply-To (and Auto-CC/BCC) from Select-All shortcut.
Comment 29•3 years ago
|
||
(In reply to Wayne Mery (:wsmwk) from comment #23)
Calum, dossy, possible for you to test Mac? https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/BsPTuFvBRr2y4dtXGr_BcA/runs/0/artifacts/public/build/target.dmg
Looks good to me on MacOS 10.15.7 (19H1323), so far.
Thanks for fixing this one.
Comment 30•3 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/6fec250b1d73
Fix macOS native context menu issue and improve the recipients area blur events. r=mkmelin
Updated•3 years ago
|
Assignee | ||
Comment 31•3 years ago
|
||
Comment on attachment 9240190 [details]
Bug 1729741 - Fix macOS native context menu issue and improve the recipients area blur events. r=mkmelin
[Approval Request Comment]
Regression caused by (bug #): bug 1700679
User impact if declined: Impossible to use the right click context menu on recipient pills with the native macos widgets.
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): medium, as it drastically changes the triggers of the context menus for all platforms, but we should be safe as new tests have been implemented to cover the entire workflow.
Assignee | ||
Comment 32•3 years ago
|
||
Comment on attachment 9240190 [details]
Bug 1729741 - Fix macOS native context menu issue and improve the recipients area blur events. r=mkmelin
[Approval Request Comment]
Regression caused by (bug #): bug 1700679
User impact if declined: Impossible to use the right click context menu on recipient pills with the native macos widgets.
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): medium, as it drastically changes the triggers of the context menus for all platforms, but we should be safe as new tests have been implemented to cover the entire workflow.
Comment 33•3 years ago
|
||
Comment on attachment 9240190 [details]
Bug 1729741 - Fix macOS native context menu issue and improve the recipients area blur events. r=mkmelin
[Triage Comment]
Approved for beta
[Triage Comment]
Approved for esr91
Comment 34•3 years ago
|
||
bugherder uplift |
Thunderbird 91.1.1:
https://hg.mozilla.org/releases/comm-esr91/rev/9626369176a3
Comment 35•3 years ago
|
||
bugherder uplift |
Thunderbird 93.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/e4aa7bc29ea5
Description
•