1783001 - Switching the active spell checking dictionary takes too many steps

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, enhancement)

Description

[[e-mail]]

Since Thunderbird 102/Bug 1761221, switching dictionary languages has become twice as much work. In addition to bringing up the language menu and selecting the desired language, you now need to access the language menu once more to disable the default language.

While it may be useful on some occasions to spellcheck multiple languages, I still mostly write emails in a single language. For this reason I propose to make a change to the language menu that allows the user to either switch or add a language. For example by holding down a modifier key for one of the two actions.

Established GUI interactions tend to use Ctrl+left-click to extend a selection and simply left-click to change the selection. But if that is too much to ask, I guess I can get used to using Shift-click or something like that to actually switch dictionary languages.

Alternatively, if there would be a setting for disabling the new feature, I would probably activate it. Writing multiple languages in a single mail is such a rare situation that it doesn't justify making the regular way of working more cumbersome, in my opinion.

Comment 1

[b1]

The menu should stay open like the thread pane column selection picker and only close when clicking outside the menu.

Comment 2

[Richard Marti (:Paenglab)]

Attachment: Bug 1783001 - Don't close the spellcheck menu on select/deselect a language. r=#thunderbird-reviewers

Comment 3

[Richard Marti (:Paenglab)]

Attachment: 1783001-spellcheck-MC.patch

This patch does the sticky thing for the context menu.
But there is one problem: with two dictionaries, one enabled the other not and then disable the first and enable the second disables all dictionaries. What is needed to work correctly?

Comment 4

[Martin Giger [:freaktechnik]]

It might be a race condition due to the entire setting of active dictionaries being async. As such, it is conceivable that it might first select only the second dictionary you selected and then none. In theory the status bar display updates after the context menu has successfully set the dictionaries, so it will reflect the latest action.

Comment 5

[Magnus Melin [:mkmelin]]

Menus staying open after click a fairly weird UI... I think like comment 0 suggests, perhaps Shift-click to select only the clicked one would be a better alternative. And/or middle-click.

Comment 6

[Alessandro Castellani [:aleca]]

(In reply to Magnus Melin [:mkmelin] from comment #5)

Menus staying open after click a fairly weird UI...

Not really.
Yes it's unusual as a general concept, but we have instances of this and we use them across the application.
Many items in the main app menu behave like that, as well as the Folder Pane menu popup, so it's not a completely foreign concept.

I think like comment 0 suggests, perhaps Shift-click to select only the clicked one would be a better alternative. And/or middle-click.

That would be a very awkward usability, non discoverable, and in general a very foreign and unique concept.

Let's not over engineer this, if multiple dictionaries are available, keeping the menu open to allow controlling the multi selection is good and intuitive.
ESC or click away will close the menu, so there's no reason to assume users will panic because a menu doesn't close after a click.
This make sense contextually.

Do we need to introduce the extra m-c patch?
Our patch of adding a closemenu="none" only if multiple dictionaries are present looks like a good solution.
Am I missing any potential technical issue?

Comment 7

[Martin Giger [:freaktechnik]]

(In reply to Alessandro Castellani [:aleca] from comment #6)

Do we need to introduce the extra m-c patch?
Our patch of adding a closemenu="none" only if multiple dictionaries are present looks like a good solution.
Am I missing any potential technical issue?

The dictionary list in the context menu is controlled by toolkit code, that's what the m-c patch is for.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/832ebfbf0ab5
Don't close the spellcheck menu on select/deselect a language. r=aleca

Comment 9

[Richard Marti (:Paenglab)]

Comment on attachment 9288754 [details]
Bug 1783001 - Don't close the spellcheck menu on select/deselect a language. r=#thunderbird-reviewers

[Approval Request Comment]
User impact if declined: more mouse clicks to change the spellcheck dictionary
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): should be low

Comment 10

[Richard Marti (:Paenglab)]

Attachment: 1783001-spellcheck-MC.patch

M-C patch with the auto hide menu when only one dictionary is installed. This patch has still the issue I described in comment 3.

Comment 11

[b1]

There were two issues in the M-C part:

1. Closure: The event listener got the initial value of curlangs even when firing a second or third time.
2. For TB this is considered as a "remote" scenario. The stored list of dictionaries is not updated if the menu doesn't close.

Also includes a few s/var/let/ changes.

This works for TB, not tested for FF. We also don't know what the non-remote code path does. We suggest to move this into a new bug and discuss with the M-C people. console.log() has been left for illustration.
https://github.com/Betterbird/thunderbird-patches/blob/main/102/bugs/1783001-dont-close-spellcheck-menu-2-m-c.patch

Comment 12

[b1]

Equivalent FF bug: Bug 1761417.

Comment 13

[Geoff Lankow (:darktrojan)]

The browser_spelling.js test is broken on macOS, and I think it's because closemenu="none" doesn't work on macOS native context menus.

Comment 14

[Richard Marti (:Paenglab)]

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

The browser_spelling.js test is broken on macOS, and I think it's because closemenu="none" doesn't work on macOS native context menus.

Where is it used in the context menu? This would be bug 1761417. My implementation is only for the spellcheck button and the status bar which don't use the native menu. Could we somehow exclude macOS from this test? I'm no test specialist.

Comment 15

[Richard Marti (:Paenglab)]

On Mac when I manually press ESC the popup closes.
I run the test now on Mac. The popup doesn't close automatically but when I press ESC the test finishes without error.

Could it be that the test doesn't apply the ESC to the popup? Is this again a Mac speciality?

Comment 16

[Wayne Mery (:wsmwk)]

Comment on attachment 9288754 [details]
Bug 1783001 - Don't close the spellcheck menu on select/deselect a language. r=#thunderbird-reviewers

[Triage Comment]
Approved for beta

Comment 17

[Rob Lemley [:rjl]]

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

Comment 18

[Wayne Mery (:wsmwk)]

Comment on attachment 9288754 [details]
Bug 1783001 - Don't close the spellcheck menu on select/deselect a language. r=#thunderbird-reviewers

[Triage Comment]
Approved for esr102 (optimistically)

Comment 19

[Geoff Lankow (:darktrojan)]

(In reply to Richard Marti (:Paenglab) from comment #15)

Could it be that the test doesn't apply the ESC to the popup? Is this again a Mac speciality?

I don't know for sure but I would think so. Try using .closePopup() instead.

Comment 20

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1783001 follow-up - Fix failing test on macOS. r=Paenglab

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2a23fbf6a40a
follow-up - Fix failing test on macOS. r=Paenglab

Comment 22

[Richard Marti (:Paenglab)]

Comment on attachment 9290079 [details]
Bug 1783001 follow-up - Fix failing test on macOS. r=Paenglab

[Approval Request Comment]
This patch should land together with the main patch to fix the test on Mac
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): should be low

Comment 23

[Rob Lemley [:rjl]]

Thunderbird 102.2.0:
https://hg.mozilla.org/releases/comm-esr102/rev/4f261a126da4
https://hg.mozilla.org/releases/comm-esr102/rev/d434be45ac53

Comment 24

[Wayne Mery (:wsmwk)]

Comment on attachment 9290079 [details]
Bug 1783001 follow-up - Fix failing test on macOS. r=Paenglab

[Triage Comment]
Approved for esr102 (already landed)