368915 - Selection of spell check dictionaries disabled in subject line

Status: VERIFIED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Karel Koubek]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: 2 pre (2007013104)

I've noticed in this build (it may be older though), that the spell checker is now active even in the subject line (in Message Compose Window). If this is a feature (which I don't favor much), then it should be possible to use the language select (Spell) button here too.

Reproducible: Always

Steps to Reproduce:
1. Write a new message
2. Type some nonsense words into the subject line 
3. 
Actual Results:  
Subject line is checked for misspellings (red underlines), but the Spell button is grayed out (inactive). If the spell checker is activated from the body of the message, then it skips the words in the subject line.


Please edit this bug accordingly, depending on whether the 'spell check in subject' is a feature or bug.

Comment 1

[Magnus Melin [:mkmelin]]

Yes, it's a feature, turned on in bug 3459. (Real good feature imo!)

But, yes, the spell button should be enabled. ->NEW

Comment 2

[Karel Koubek]

Thanks for clarification, Magnus.

Having read the original bug 3459, it seems that the subject and message body spell-checkers are independent (at least for now). So it won't be possible to press the Spell button in one text area and then go through all the misspelled words (including the words from the other text area), which is the preferred behavior.

In such a situation, the Spell button pressed in subject should go through all the misspelled words in this line and then say 'Completed Spell checking.' as it does when spell checking is finished in the message body.
It should be also possible to change the dictionary here (via the Spell drop-down), but then it must also update the dictionary settings of the message body spell checker.

Hope it's not so hard to do, so it can make it into the version 2 final.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

this is still an issue in 2.0.0.17 on windows XP

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

some more data where the subject line and body do not work in sync with the spell checker:
 - Subject line is missing "ignore word"
 - If you "Add a word to dictionary" in the body, it is still underlined in subject line, but no where else in the body
   - if you type the word after it is in the dictionary in the subject line, it won't be underlined
 - if you add a mispelled word from the subject line to dictionary, it doesn't apply to the body
   - if you type the newly added word from the subject line in the body, it is recognized as a valid word.

Comment 5

[Wayne Mery (:wsmwk)]

Joel, your comment 4 is  Bug 433181

Comment 6

[Thomas D. (:thomas8)]

We don't have an entry for the spell checking shortcut in Thunderbird Keyboard Shortcuts page (http://www.mozilla.org/support/thunderbird/keyboard#), but looking at Options > Check spelling... (and tested working in body) the current shortcut seems to be Ctrl+Shift+P.

Comment 7

[Thomas D. (:thomas8)]

I'd love to fix this, and I suppose it's dead-simple, but I searched all over the place without luck. Can someone give me a hint how to approach this? (which files to look at, how to enable)

Comment 8

[David von Oheimb]

Thomas, I'd be glad if you can fix this; 
I suggest having a look at: mozilla/extensions/spellcheck/src/

Comment 9

[Thomas D. (:thomas8)]

I'm afraid I currently don't have time for this, and not enough knowledge. David, within the spellcheck/src directory, would you know which exact files we would have to fix for this?

Comment 10

[David von Oheimb]

No, sorry. I do not have time for this, either.

Comment 11

[Magnus Melin [:mkmelin]]

I think it's more likely around here:
http://mxr.mozilla.org/comm-central/source/editor/ui/dialogs/content/EdSpellCheck.js#120 - but the fix doesn't look trivial

Comment 13

[Michael Poczynek]

Considering the bug was reported 3 years ago, and is still not fixed.  What is the point to this forum anyway?

Comment 14

[Quentin Headen [:qheaden]]

Hmm, this is an ancient bug, but I'm going to see if I can possibly do anything with it.

Comment 15

[Jim Porter (:squib)]

(In reply to Quentin Headen from comment #14)
> Hmm, this is an ancient bug, but I'm going to see if I can possibly do
> anything with it.

CC'ing protz for this; I recall he did some work with spellcheck recently, so he might be able to give you some pointers.

Comment 16

[David von Oheimb]

Glad that the bug is finally tackled again. Note that it is just a GUI issue.
When the message body field is active, changing the language there also affects the spell setting of the subject field. So the only thing needed is to enable the language selector when the subject field is active.

Comment 17

[Jonathan Protzenko [:protz]]

Now you can right-click in the subject field and choose the language from the context menu. I think that's enough, isn't it?

Comment 18

[Jonathan Protzenko [:protz]]

If someone wants to make sure the toolbar button can switch the language of the subject field, the file to look for is mail/components/compose/src/MsgComposeCommands.js I think...

Comment 19

[Quentin Headen [:qheaden]]

I compiled the latest Thunderbird code and I can confirm that this bug still exists.  I'm going to try to work on it. Can someone assign me to it?

Thanks

Comment 20

[Jim Porter (:squib)]

(In reply to Jonathan Protzenko [:protz] from comment #18)
> If someone wants to make sure the toolbar button can switch the language of
> the subject field, the file to look for is
> mail/components/compose/src/MsgComposeCommands.js I think...

I believe that's "mail/components/compose/content/MsgComposeCommands.js" (src/ is usually for C++ code).

Comment 21

[Quentin Headen [:qheaden]]

Could someone please point me to the code that enables/disables the spellchecking button when the message body textbox goes in and out of focus?

Thanks.

Comment 22

[Thomas D. (:thomas8)]

Ludo, who can point us to the relevant code here?

(In reply to Quentin Headen from comment #21)
> Could someone please point me to the code that enables/disables the
> spellchecking button when the message body textbox goes in and out of focus?
> Thanks.

I tried a couple of things, searching for spellingButton, cmd_spelling etc., but I couldn't find where / how the button gets disabled. It's disabled by default, so maybe it gets disabled by refreshing the whole UI when we move focus into body?

Comment 23

[Jim Porter (:squib)]

(In reply to Thomas D. from comment #22)
> Ludo, who can point us to the relevant code here?

I already discussed this with Quentin on IRC. The way to go here is probably to create a command controller for the subject field and handle the cmd_spelling command there.

For what it's worth, here's the code the message body uses: <http://mxr.mozilla.org/comm-central/source/editor/ui/composer/content/ComposerCommands.js#2425>

Comment 24

[Quentin Headen [:qheaden]]

I really, really hate to do this, but I am unassigning myself from this bug. It is much harder to fix than I originally thought. Also, since I am new to the Mozilla project, I don't have sufficient knowledge of XUL and Javascript debugging to really inspect the cause of this issue.

Hopefully someone else can handle it better than I. :)

Comment 26

[Jorg K (CEST = GMT+2)]

TB 17.0.8:

When starting a new message the "Spelling" button (the one next to the "Send" button) is disabled. One has to click into the message body first, before one can select the language. Then one can start typing the subject and the body.

Normal would be:
1) Select language
2) Type subject
3) Type message body.

Comment 27

[Jorg K (CEST = GMT+2)]

This is still an issue in 24 and 31. As described in comment #26, the "spelling" button is greyed out unless the cursor is positioned on the text body. This makes little sense, since the subject of the messages needs spell-checking as well.

Only good thing: The subject line does get spell checked when the spelling dictionary is changed, same behaviour in 24 and 31.

Comment 28

[Jorg K (CEST = GMT+2)]

Now that bug 967494 (and bug 378434) is being fixed, would it be possible to change the UI so that the "Spelling" button is available when the cursor is in the subject field.

Comment 29

[Jan Horak [:jhorak]]

I'll look into it when fix for bug 967494 land.

Comment 30

[Jan Horak [:jhorak]]

There's really no easy fix for this issue. Comment 2 is very valid here. Fixing this would need to join spellchecker for subject and body and spellchecker was designed only to check one 'editor' at a time (subject and body has independent editors).

Comment 31

[Jorg K (CEST = GMT+2)]

Well, even if there are two different editors at work, why must the "Spelling" button be disabled, so one can't change the language while the cursor is in the subject field?

Comment 32

[Jan Horak [:jhorak]]

(In reply to Jorg K from comment #31)
> Well, even if there are two different editors at work, why must the
> "Spelling" button be disabled, so one can't change the language while the
> cursor is in the subject field?

Because click on button launch spellchecking window. Changing the language is somehow additional and the button is disabled because code is far from being able to start spellchecking of subject and body together.

Comment 33

[Jorg K (CEST = GMT+2)]

OK. And it could not just spell check the subject?
Users at that stage don't want to spell check anything. They want to set the language for the subject and e-mail body they are about to type.

Comment 34

[Alain Knaff]

(In reply to Jorg K from comment #33)
> OK. And it could not just spell check the subject?
> Users at that stage don't want to spell check anything. They want to set the
> language for the subject and e-mail body they are about to type.

Ironically enough, it _does_ actually spell check the subject... the only thing that isn't working is choosing the language while typing the subject. For that you have to click into the body, chose language, and this language then also applies to subject...

Comment 35

[Alain Knaff]

(In reply to Jan Horak from comment #32)
> (In reply to Jorg K from comment #31)
> > Well, even if there are two different editors at work, why must the
> > "Spelling" button be disabled, so one can't change the language while the
> > cursor is in the subject field?
> 
> Because click on button launch spellchecking window.

A ok... never noticed that before. I usually only use that button to change language (as a Luxembourger, there are 4 different languages that I regularly use... so switching is not a rare occurrence...)

> Changing the language
> is somehow additional and the button is disabled because code is far from
> being able to start spellchecking of subject and body together.

Spellchecking (i.e. underlining misspelled words in subject and body text itself, and right-clicking misspelled word for suggestions) does actually work (and it useful). Maybe that separate window would not be working, but I rarely use that one.

Maybe, rather than making the language-selection feature "additional" to that button, just make it its main function, and make the "spell-checking window" the "additional" thing. I.e. the spell-checking window will open up when it can, and just pop up a message saying it can't when it can't. That way, people who only care about language selection can get it, wherever their text focus happens to be.

Comment 36

[Jorg K (CEST = GMT+2)]

I fully agree. I never use the "Spelling" button for a spell check, only to set the language.
BTW, not only Luxembourgers use four languages regularly, I as a Berliner do, too!

Comment 37

[David von Oheimb]

I see this bug as pretty minor (as compared to other major bugs that are still around for years),
since the on-the-fly spell checking does take place automatically, or at least can be triggered by right-clicking into the text and changing the language (assuming a second language having been installed).

The only minor complaint I have here is that when switching the language to English for the body or for the subject, the red underlines change accordingly for the subject (or body, respectively) as well, which is what I'd expect, but:
When switching from English to German in either the body or subject and then left-click to focus the other field (i.e., subject or body, respectively), the language reverts automatically for both fields.

Comment 38

[Jan Horak [:jhorak]]

(In reply to David von Oheimb from comment #37)
> I see this bug as pretty minor (as compared to other major bugs that are
> still around for years),
> since the on-the-fly spell checking does take place automatically, or at
> least can be triggered by right-clicking into the text and changing the
> language (assuming a second language having been installed).
> 
> The only minor complaint I have here is that when switching the language to
> English for the body or for the subject, the red underlines change
> accordingly for the subject (or body, respectively) as well, which is what
> I'd expect, but:
> When switching from English to German in either the body or subject and then
> left-click to focus the other field (i.e., subject or body, respectively),
> the language reverts automatically for both fields.
This should be addressed in bug 967494.

Comment 39

[Jorg K (CEST = GMT+2)]

Now that bug 967494 has landed, can someone point me to the code that disables spell checking while the cursor is in the subject line. I only found:
MsgComposeCommands.js
function enableInlineSpellCheck(aEnableInlineSpellCheck)
{
  gSpellChecker.enabled = aEnableInlineSpellCheck;
  document.getElementById('msgSubject').setAttribute('spellcheck', aEnableInlineSpellCheck);
but that doesn't seem to switch if off.

Remember what Magnus said in comment #1:
> But, yes, the spell button should be enabled. ->NEW

Kent: Haha, this is really the very last bug I'll nag about ;-)

Comment 40

[[e-mail]]

(In reply to Jorg K from comment #39)
> Now that bug 967494 has landed, can someone point me to the code that
> disables spell checking while the cursor is in the subject line.
You've got it back-to-front.

Composer (editor/ui) knows about spell checking in editable frames. So when the cursor is put in the body, the spelling button says "Can you spell?" and Composer says "yes". (Then when you click the button Composer opens its spelling dialog.)

When the cursor is anywhere else, the spelling button says "Can you spell?" but gets no reply. So it disables itself, since it doesn't know what to do when you click it.

Comment 41

[Jorg K (CEST = GMT+2)]

Thanks! Do you have a line number for me? Would it be hard to enable the spell button so one can change the dictionary whilst in the subject? Or not desired? If it's a two-liner, perhaps you can submit a patch, I'm sure you can do it faster than me, besides, in the UK it's one hour earlier still ;-)
Would be nice to push this into TB38.

Comment 42

[[e-mail]]

(In reply to Jorg K from comment #41)
> Thanks! Do you have a line number for me?
Line number for what?

> Would it be hard to enable the
> spell button so one can change the dictionary whilst in the subject?
It's not a two-liner.

Comment 43

[Jorg K (CEST = GMT+2)]

(In reply to [e-mail] from comment #42)
> (In reply to Jorg K from comment #41)
> > Thanks! Do you have a line number for me?
> Line number for what?

Line number for where the composer says "yes".

Comment 44

[[e-mail]]

SetupTextEditorCommands() registers a number of commands with the command table. Then when you update the button's state, goUpdateCommand locates the active command only if the body is focused.

Comment 45

[Jorg K (CEST = GMT+2)]

Too ambitious for TB38.0, hopefully we can fix it at some stage in ESR 38.

Comment 46

[Kent James (:rkent)]

I don't want to use tracking-thunderbird_esr38 for "hope to fix" bugs. You are welcome to advocate for the bug to be fixed, and nominate it for approval once fixed. Regressions from TB 31 may be appropriate for tracking.

Comment 47

[Jorg K (CEST = GMT+2)]

Attachment: WIP for discussion

The problem of this bug is, that the selection of the dictionary is coupled to the spell check in the same button. Why?

Wouldn't it be the simplest solution to have two independent UI elements, one for spell checking as before, and a new one, that lets you change the dictionary?

This very simple solution works, only that the new "button" named "spellingDicts" shows with a wide empty label.

Is there another element that could be used to generate just the dropdown menu or how could I just remove the label or shrink it to zero width?

Comment 48

[Jorg K (CEST = GMT+2)]

I've done a bit of experimenting and changed the "Spelling" button so it's always enabled. However, when clicking it, it always spells the message body, never the subject.

So I'm not sure how the desired solution should look like:
1) Full spell check of the subject and the message body or
2) Full spell check in the message body and the ability to set the language for
   inline spell checking whilst in the subject field.

I'd be happy with the second "light weight" option. The first option is a lot of work for little gain. Looks like comment #35 agrees with me.

Let's ask the reporter of the bug.

Comments are of course welcome.

Comment 49

[Jorg K (CEST = GMT+2)]

Of course the whole issue is not urgent any more, since you can right-click in the subject and chose the spelling language this way. Since we fixed bug 717292, subject and body language will always be synchronised.

Comment 50

[[e-mail]]

Comment on attachment 8613234 [details] [diff] [review]
WIP for discussion

>+    <toolbarbutton class="toolbarbutton-1" type="menu-button"
You want type="menu" here.

Comment 51

[Jorg K (CEST = GMT+2)]

Attachment: Looks worse with type="menu"

Thanks for the feedback+.

Using type="menu" loses the separator, which I wanted to maintain, and gives an equally wide empty box.

I wanted to maintain the original look, so
  Spelling|v
where v is the down-arrow that pops up the dictionary menu.

Comment 52

[Magnus Melin [:mkmelin]]

Comment on attachment 8613234 [details] [diff] [review]
WIP for discussion

Review of attachment 8613234 [details] [diff] [review]:
-----------------------------------------------------------------

Can't say about the UI as it shows nothing at all on linux.

On principle I do agree people almost always use that button to change dictionary. Who really use anything but inline spell checking?
But on the other hand, having a button that doesn't do anything but select language would be very odd too.

Comment 53

[Jorg K (CEST = GMT+2)]

(In reply to Magnus Melin from comment #52)
> But on the other hand, having a button that doesn't do anything but select
> language would be very odd too.

That's why I wanted to "cut" the existing button
 [Spelling|v]
in half into
 [Spelling]
and
         [|v].
I think I have enough positive feedback to pursue this further, that is, make it look the same as the old interface, only that the down-arrow is no longer part of the button and won't be disabled.

Comment 54

[Magnus Melin [:mkmelin]]

Well, you could also just have the button always enabled. If you really hadn't entered any text, you'd just get a "no misspelled words" shown anyway.

Comment 55

[Jorg K (CEST = GMT+2)]

As I said in comment #48: With the button always enabled, the message body is checked, not the subject.

I don't want to make the decision to change the UX. Perhaps Neil can said something about this. I think the idea of UX design is to show the user the elements he can use and not the ones he can't use. But then, there's another train of thought that says, users will wonder why an element is greyed out and it's better to give them a message.

I don't care either way.

Comment 56

[Jorg K (CEST = GMT+2)]

Attachment: WIP: Patch to leave the spelling button always enabled.

With this, /mail/components/compose/content/ registers something defined in /editor/ui/composer/content/. Maybe this is a problem.

I tried registering it in SetupComposerWindowCommands instead,
https://dxr.mozilla.org/comm-central/source/editor/ui/composer/content/ComposerCommands.js#144 
but that doesn't seem to be called, maybe it's there for SeaMonkey (surely there is no "Publish" command in Thunderbird).

Comment 57

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons that look like the one we had before

I personally like the approach better of *not* leaving the spell button always enabled.

With the little tweak I'm proposing, we make a new button that just shows the arrow to allow the selection of the dictionary.

Left border colour: looks OK in Windows.
For OS X, hsla(0, 0%, 0%, .2); seems to be used.
For Linux, there is no separator used.

Comment 58

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons that look like the one we had before (v2)

Actually, the button needs a label, otherwise it doesn't show up during the customisation of the toolbar (when dragging the little arrow to the customisation window).

Comment 59

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons that look like the one we had before (v3)

Much better solution.
Now we have two buttons "Spelling" and "Language" which are independent and which work fully when customising the toolbar.
The trick is, that when "Language" follows "Spelling" on the toolbar, it loses the icon and the label, so it looks exactly like one button.

Comment 60

[Magnus Melin [:mkmelin]]

Comment on attachment 8616442 [details] [diff] [review]
Proposed patch to have two independent buttons that look like the one we had before (v3)

Review of attachment 8616442 [details] [diff] [review]:
-----------------------------------------------------------------

It's a bit odd, but works surprisingly well. 
Since default buttons aren't migrated, I'd be a little bit concerned users won't find this though. 

Semantically a bit wrong, but maybe we should reuse the spellingButton id for language setting, and instead have the other button for full spell checking, as that's probably not that much used?

::: mail/components/compose/content/messengercompose.xul
@@ +786,5 @@
>                 tooltiptext="&spellingButton.tooltip;"
>                 command="cmd_spelling">
> +    </toolbarbutton>
> +    <toolbarbutton class="toolbarbutton-1" type="menu"
> +               id="spellingLanguage" label="&spellingLanguage.label;" tooltiptext="" command="">

please put id (and the others on this row) first. you can reorder the spellingButton attributes too if you want.

::: mail/themes/linux/mail/compose/messengercompose.css
@@ +73,5 @@
> +#spellingLanguage {
> +  list-style-image: url("moz-icon://stock/gtk-spell-check?size=toolbar");
> +}
> +
> +/* If the dictionary button follows the spelling button, we change the style and hide text and label. */

probably the other way around too

Comment 61

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons that look like the one we had before (v4)

Carrying forward Magnus' review+ after fixing the nits.

> please put id (and the others on this row) first. you can reorder the 
> spellingButton attributes too if you want.
The order seems to be a little inconsistent in the file. Now I put:
id= class= type=, that's the CSS stuff. Then label= tooltiptext=, that the UX stuff.
I added a tooltip text.

> probably the other way around too
Comment now reads:
If the language button follows the spelling button, we change the style and hide text and label.

This reflects correctly what is happening.

Comment 62

[Magnus Melin [:mkmelin]]

Comment on attachment 8616490 [details] [diff] [review]
Proposed patch to have two independent buttons that look like the one we had before (v4)

Review of attachment 8616490 [details] [diff] [review]:
-----------------------------------------------------------------

What I meant was that in any combination that the buttons follow each other they should probably merge. 

Please get ui-r too though.

::: mail/components/compose/content/messengercompose.xul
@@ +787,5 @@
>                 command="cmd_spelling">
> +    </toolbarbutton>
> +    <toolbarbutton id="spellingLanguage" class="toolbarbutton-1" type="menu"
> +               label="&spellingLanguage.label;"
> +               tooltiptext="&languageButton.tooltip;">

nit: align attributes

Comment 63

[Magnus Melin [:mkmelin]]

(In reply to Magnus Melin from comment #60)
> Semantically a bit wrong, but maybe we should reuse the spellingButton id
> for language setting, and instead have the other button for full spell
> checking, as that's probably not that much used?

We should figure this out too.

Comment 64

[Magnus Melin [:mkmelin]]

Comment on attachment 8616490 [details] [diff] [review]
Proposed patch to have two independent buttons that look like the one we had before (v4)

Review of attachment 8616490 [details] [diff] [review]:
-----------------------------------------------------------------

What I meant was that in any combination that the buttons follow each other they should probably merge. 

Please get ui-r too though.

::: mail/components/compose/content/messengercompose.xul
@@ +787,5 @@
>                 command="cmd_spelling">
> +    </toolbarbutton>
> +    <toolbarbutton id="spellingLanguage" class="toolbarbutton-1" type="menu"
> +               label="&spellingLanguage.label;"
> +               tooltiptext="&languageButton.tooltip;">

nit: align attributes

Comment 65

[Jorg K (CEST = GMT+2)]

(In reply to Magnus Melin from comment #60)
> Since default buttons aren't migrated, I'd be a little bit concerned users
> won't find this though. 

You're saying that the new button won't show up automatically? It did for me.
Users who want the feature will go looking, don't you think?

I'd rather *not* call the "language selector" spellingButton and change the existing spellingButton to spellCheckButton.

Comment 66

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons that look like the one we had before (v5)

Fixed the alignment.

Note to the UX reviewer:
The "Spelling" button which also contains the language choice is disabled when in the subject line. While having a context-sensitive button is a good thing, disabling the language choice is a bad thing.

My solution is to "split" the button into two buttons, which, when placed one after the other, behave like the previous button, only that the right part, the language selector, is no longer disabled.

Comment 67

[[e-mail]]

(In reply to Jorg K from comment #56)
> Created attachment 8615925 [details] [diff] [review]
> WIP: Patch to leave the spelling button always enabled.
> 
> With this, /mail/components/compose/content/ registers something defined in
> /editor/ui/composer/content/. Maybe this is a problem.
Only the editor change is a problem; I think the controller in MsgComposeCommands listens at a higher level than the one in ComposerCommands so they don't conflict; if the focus is in the body then the editor version wins otherwise the mail one does. (So you could make the mail one do different things or nothing at all for instance.)
[skipBlockQuotes is always true here of course.]

> I tried registering it in SetupComposerWindowCommands instead,
> https://dxr.mozilla.org/comm-central/source/editor/ui/composer/content/
> ComposerCommands.js#144 
> but that doesn't seem to be called, maybe it's there for SeaMonkey (surely
> there is no "Publish" command in Thunderbird).
Right, there's a controller for the code editing functionality and then separate controllers for the things specific to the composer and message compose windows.

Comment 68

[Jorg K (CEST = GMT+2)]

Neil: Thanks for your reply. As you saw, I'm not pursuing the "always enabled" solution (attachment 8615925 [details] [diff] [review]) any more. I hope we can get the "split button" solution approved. You gave the idea feedback+ in comment #50, please also give feedback+ to the latest version. Magnus seemed to approve as well, with the review+ from comment #60.

Comment 69

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons that look like the one we had before (v6)

Fiexed bit-rotted after landing of bug 1150627 (SVG images instead of PNG).

Comment 70

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons that look like the one we had before (v7)

Oops, in my hasty refresh I got the left border colours for the "separator" between icon/label and the little arrow wrong (cut/paste error). Now they are as intended again:

Windows (Aero):
border-left: solid 1px hsla(210, 54%, 20%, .35);

Windows XP and Linux: There are no "separators" between icon/label and arrow.
border-left: solid 1px transparent;

OS X:
border-left: solid 1px hsla(0, 0%, 0%, .2);

Comment 71

[Magnus Melin [:mkmelin]]

Comment on attachment 8621489 [details] [diff] [review]
Proposed patch to have two independent buttons that look like the one we had before (v7)

Review of attachment 8621489 [details] [diff] [review]:
-----------------------------------------------------------------

I'm fine with this. r=mkmelin

Comment 72

[Jorg K (CEST = GMT+2)]

Thanks, Magnus. So we wait for UI review, right?

The "executive summary" for the UI reviewer can be found in comment #66.

Comment 73

[Magnus Melin [:mkmelin]]

Yep.

Comment 74

[Blake Winton (:bwinton) (:☕️)]

Could the UI reviewer get screenshots on Windows and Linux, so that he doesn't have to boot into three different operating systems to see what it looks like?  ;)

(I'll do the Mac build now.)

Comment 75

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 8621489 [details] [diff] [review]
Proposed patch to have two independent buttons that look like the one we had before (v7)

I like the idea.  The implementation on Mac doesn't look enough like a split button to get the ui-r+, though.

(I'm also a little worried that people will assume the whole button is disabled, instead of just the left-hand side, but I _think_ I could live with that since we don't offer a better way to change the language.)

Comment 76

[Blake Winton (:bwinton) (:☕️)]

Wait, no, I think I take it back.  Customizing is just too confusing.  I think it would be better to have "Spelling" and "Spelling+Language" as two different buttons, where the second one has the spell-check button plus the combo-dropdown.

Comment 77

[Jorg K (CEST = GMT+2)]

Attachment: This is how it looks on Windows 7 (Aero).

Hi UI-reviewer, hi Blake, we haven't met before.

Sadly my build doesn't run on XP, but I could look into it if necessary. I hope Magnus can give us some Linux screenshots.

The difference between the different versions is the line (left border) as detailed in comment #70.

Comment 78

[Jorg K (CEST = GMT+2)]

(In reply to Blake Winton (:bwinton) from comment #75)
> I like the idea.  The implementation on Mac doesn't look enough like a split
> button to get the ui-r+, though.

How does it look like, can you attach an image.

(In reply to Blake Winton (:bwinton) from comment #76)
> Wait, no, I think I take it back.  Customizing is just too confusing.  I
> think it would be better to have "Spelling" and "Spelling+Language" as two
> different buttons, where the second one has the spell-check button plus the
> combo-dropdown.

We currently do have a "Spelling+Language" button. The problem is that the entire UI element is disabled, so when now "Spelling" is available, no "Language change" is available.

So the options to solve this are:
1) Make it two buttons that look like one, as I have done.
2) Always enable the existing "Spelling+Language" button. That's ugly, since when you press it while in the subject, you still check the message body.
3) Invest a whole lot of work to enable the existing "Spelling+Language" button while in the subject. This is still no good, since pressing the button would spell-check the body.
3+) Invest even more work to implement a (non-inline) spell-check in the subject.

The little tweak we're after doesn't warrant 3) or 3+) so we're left with 1) or 2) ... or not do anything at all.

Comment 79

[Jorg K (CEST = GMT+2)]

Sorry:
..., so when NO "Spelling" is available, NO "Language change" is available.

Comment 80

[Jorg K (CEST = GMT+2)]

Attachment: With no tooltip. Looks like the button we always had. Only that the right side is always enabled.

Comment 81

[Jorg K (CEST = GMT+2)]

(In reply to Blake Winton (:bwinton) from comment #75)

> I like the idea.  The implementation on Mac doesn't look enough like a split
> button to get the ui-r+, though.
So where we go from here? Please tell me what exactly you don't like about the look on Mac and consider the options outlined in comment #78. Also please clarify what you meant by:
> I think it would be better to have "Spelling" and "Spelling+Language" as two 
> different buttons, where the second one has the spell-check button plus the combo-dropdown.
This makes no sense to me. Why have two buttons solve anything? Also, the "Spelling+Language" button you talk about already exists. Only that we can't drive the two parts independently.

Comment 82

[Blake Winton (:bwinton) (:☕️)]

(In reply to Jorg K from comment #81)
> (In reply to Blake Winton (:bwinton) from comment #75)
> > I like the idea.  The implementation on Mac doesn't look enough like a split
> > button to get the ui-r+, though.
> So where we go from here? Please tell me what exactly you don't like about
> the look on Mac and consider the options outlined in comment #78. Also

So, let me post a screenshot…
https://dl.dropboxusercontent.com/u/2301433/Screenshots/SpellingButton.png

Comparing the button to the "Attach" button beside it, on Mac, as you can see, the line separating the two is far too tall; the arrow is way too close to the separator; the hover style on the Spelling button or the dropdown arrow doesn't cover the whole button; and the hover style on the dropdown arrow doesn't have rounded corners.

> please clarify what you meant by:
> > I think it would be better to have "Spelling" and "Spelling+Language" as two 
> > different buttons, where the second one has the spell-check button plus the combo-dropdown.
> This makes no sense to me. Why have two buttons solve anything? Also, the
> "Spelling+Language" button you talk about already exists. Only that we can't
> drive the two parts independently.

So, I guess I wasn't really being clear there…  But that's okay, since because of your comment I've taken a little more time to play around with the button, and I think I have a better solution.

Always enable the Spelling button, but if we're not in the body, have clicks on it just return without checking anything.
(So, like option 2, but without the ugliness of always checking the body.)

How does that sound to you?

Comment 83

[Jorg K (CEST = GMT+2)]

(In reply to Blake Winton (:bwinton) from comment #82)
> So, let me post a screenshot…
> https://dl.dropboxusercontent.com/u/2301433/Screenshots/SpellingButton.png
Thanks. BTW, why didn't you attach the image to the bug, so we have a permanent copy? I'll do it now.
The picture shows two sets of buttons, I assume they are from two different versions of OS X or two different themes. Please confirm. I actually don't understand where the second set comes from, since the Mac stuff I've seen (also attached) looks like your first set.
First set: Yes, the separator line is a little too tall, that could be fixed in CSS. The arrow is a little to close, again, this can be fixed in CSS. On Windows, the separator is a little taller than the separator of the attach button, but then, between the send and the spelling button, there is a "real" separator, which has about the same hight. On Windows, I counted the pixels between line and arrow, four pixels for the new "spelling combo" and also four pixels for the attach button.
Second set: Yes, I can see that the appearance is different from the attach button.

Of course, hovering over the "spelling combo" is different from hovering over the attach button, since the two controls react independently, the arrow has its own tooltip. That's how the user will notice the new feature, which is otherwise almost invisible.

I can see that the second set on OS X is not so pretty, the first set could be fixed. That together with the somewhat surprising customisation makes the approach questionable for Mac users.

> Always enable the Spelling button, but if we're not in the body, have clicks
> on it just *return* without checking anything.
> (So, like option 2, but without the ugliness of always checking the body.)
> How does that sound to you?
Frankly, that's terrible UI design (with all due respect to the UX team leader - https://wiki.mozilla.org/Thunderbird:UX/Team).
I thought UI design 101 teaches: a) disable the element that the user can't use (and let them wonder why they can't use it) or b) let them us it and give them an error. Or in this case: let them use it and show them that they shouldn't have used it by doing something unexpected, like spelling the body while pressing the button in the message subject. Letting the user push the button and not do anything (that's what you meant by 'return' I assume) is a big "no no".

I'm at the point of abandoning this bug. This bug is a little tweak to allow people to select the language *before* they start typing the subject (which is spell checked, so you get red underlines if you're in the wrong language). Currently the user has to click in the message body, then change the language, then return to the subject, which is a terrible user interface.

I invested countless hours to investigate how the CSS needs to be modified to make the icon and label disappear then the two controls are next to one another (yes, the solution is simple, but finding it was not so simple). I had to un-bit-rot my patch once, since in bug 1150627 the PNGs were swapped for SVGs, and now the damn OS X comes along which does everything differently and tramples right over the otherwise quite feasible solution. That's just too much effort for too little gain. I think it would be a better idea to offer this as an add-on of two buttons that users can use to replace the existing "spelling+language" button, and if it's ugly on the Mac, tough!

Comment 84

[Jorg K (CEST = GMT+2)]

Attachment: Two sets of buttons on Mac OS X supplied by Blake Winton

Comment 85

[Jorg K (CEST = GMT+2)]

Attachment: Sceenshot of toolbar on Mac OS X from Jorg K's collection

This is the information I had to see whether the solution could work in Mac OS X. Sadly there seems to be another "mode/theme", where buttons have rounded edges and extra space is inserted between controls.

Comment 86

[Jorg K (CEST = GMT+2)]

Attachment: Suggestions for two truly independent buttons, no jiggery-pokery

There is of course a very simple solution:
Have an independent "Language" buttons that doesn't try to amalgamate with the "Spelling" button. That's clear cut, single-language users who never change the language can simply remove it.
On the button you could (try to) display the currently selected language. The question is, which icon to use. The attachment shows a mock-up.

Comment 87

[Blake Winton (:bwinton) (:☕️)]

(In reply to Jorg K from comment #83)
> (In reply to Blake Winton (:bwinton) from comment #82)
> The picture shows two sets of buttons, I assume they are from two different
> versions of OS X or two different themes. Please confirm. I actually don't
> understand where the second set comes from, since the Mac stuff I've seen
> (also attached) looks like your first set.

It's the same theme, but in the bottom one, each of the buttons is hovered over.
(So you would never see that whole row, but on hover, each of the top buttons would change to look like the bottom version.)

> First set: Yes, the separator line is a little too tall, that could be fixed
> in CSS. The arrow is a little to close, again, this can be fixed in CSS. On
> Windows, the separator is a little taller than the separator of the attach
> button, but then, between the send and the spelling button, there is a
> "real" separator, which has about the same hight. On Windows, I counted the
> pixels between line and arrow, four pixels for the new "spelling combo" and
> also four pixels for the attach button.

It should all be fixed in CSS, yes, which is what I was trying to indicate with the ui-r-.

> Of course, hovering over the "spelling combo" is different from hovering
> over the attach button, since the two controls react independently, the
> arrow has its own tooltip. That's how the user will notice the new feature,
> which is otherwise almost invisible.

That seems different than every other split button, which makes me think that maybe this isn't the best idea.

> I can see that the second set on OS X is not so pretty, the first set could
> be fixed. That together with the somewhat surprising customisation makes the
> approach questionable for Mac users.

The second set would also need to be fixed to get a ui-r+, I'm afraid.  But I agree that it seems questionable for Mac users.

> > Always enable the Spelling button, but if we're not in the body, have clicks
> > on it just *return* without checking anything.
> > (So, like option 2, but without the ugliness of always checking the body.)
> > How does that sound to you?
> Frankly, that's terrible UI design (with all due respect to the UX team
> leader - https://wiki.mozilla.org/Thunderbird:UX/Team).

That seems needlessly insulting.

> I thought UI design 101 teaches: a) disable the element that the user can't
> use (and let them wonder why they can't use it) or b) let them use it and
> give them an error.

It does, but it also teaches us: Don't have the same button do two different things, and Don't make buttons act wildly differently than the similar looking buttons they're placed beside, and many other things…

> Or in this case: let them use it and show them that they
> shouldn't have used it by doing something unexpected, like spelling the body
> while pressing the button in the message subject. Letting the user push the
> button and not do anything (that's what you meant by 'return' I assume) is a
> big "no no".

There seem to be a lot of "no no"s in the current interface.  This decision seems to be a matter of picking the least-bad alternative.  The actual best design (having the button check the spelling of the field you've highlighted) was dismissed as too hard, so we're left with choosing between terrible alternatives.

> I invested countless hours to investigate how the CSS needs to be modified
> to make the icon and label disappear then the two controls are next to one
> another (yes, the solution is simple, but finding it was not so simple). I
> had to un-bit-rot my patch once, since in bug 1150627 the PNGs were swapped
> for SVGs, and now the damn OS X comes along which does everything
> differently and tramples right over the otherwise quite feasible solution.

As someone who's written many patches for the front-ends of both Firefox and Thunderbird, I totally sympathize with you.  It's confusing and largely undocumented, and platform-specific in bizzare ways.  This is why I was suggesting using the built-in styles to have a normal split-button that works like every other split button on all the platforms.  Yes, the always-enabled behaviour is slightly confusing (although perhaps, instead of not doing anything, it could open the dropdown when you're not in the body?), but at least it would look and act like all the other split-buttons in the interface…

> That's just too much effort for too little gain. I think it would be a
> better idea to offer this as an add-on of two buttons that users can use to
> replace the existing "spelling+language" button, and if it's ugly on the
> Mac, tough!

As long as you use the built-in styles, it should look fine on the Mac.
(And saying "Tough!" to 10% of our users isn't a great idea…)

(In reply to Jorg K from comment #86)
> Created attachment 8622948 [details]
> Suggestions for two truly independent buttons, no jiggery-pokery
> 
> There is of course a very simple solution:
> Have an independent "Language" buttons that doesn't try to amalgamate with
> the "Spelling" button. That's clear cut, single-language users who never
> change the language can simply remove it.
> On the button you could (try to) display the currently selected language.
> The question is, which icon to use. The attachment shows a mock-up.

This does seem simpler and more understandable than the other options we're considering.  I suggest stealing the "Character Encoding" icon from Firefox, as shown here: https://www.dropbox.com/s/scvpv0h14xdfs5v/Screenshot%202015-06-16%2010.01.39.png?dl=0
The source file is at http://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/[e-mail] (and other, similar, places).  I would support landing this in Thunderbird.

Thanks,
Blake.

Comment 88

[Jorg K (CEST = GMT+2)]

(In reply to Blake Winton (:bwinton) from comment #87)
> It's the same theme, but in the bottom one, each of the buttons is hovered
> over.
Thanks for the clarification.

> That seems needlessly insulting.
I'm sorry. No offence intended. I shouldn't offend you since you will need to sign off on my stuff ;-) Written communication always has the risk of coming across as too harsh. I just thought a button that did nothing was a really bad idea, but as you say further on:
> This decision seems to be a matter of picking the least-bad alternative.

> The actual best design (having the button check the spelling of the field you've
> highlighted) was dismissed as too hard, so we're left with choosing between
> terrible alternatives.
You're right, my laziness. It would mean to
a) leave the button context sensitive, but enable it for subject and body.
b) Implement the spell checking for the subject.
It appears too much work, since IHMO which is also shared by Magnus (comment #52 "Who really uses anything but inline spell checking?"), the "Spelling" button is not used much, and to implement a spell check for the one line subject is too much effort.

> Yes, the
> always-enabled behaviour is slightly confusing (although perhaps, instead of
> not doing anything, it could open the dropdown when you're not in the
> body?), but at least it would look and act like all the other split-buttons
> in the interface…
Hmm, yes, maybe the dropdown.

> (And saying "Tough!" to 10% of our users isn't a great idea…)
This was in the context of an "add-on". And the add-on could break all the rules, since no one is obliged to use it. But of course the idea of having it work out of the box is better.

> > Have an independent "Language" buttons that doesn't try to amalgamate with
> > the "Spelling" button. That's clear cut, single-language users who never
> > change the language can simply remove it.
> This does seem simpler and more understandable than the other options we're
> considering.  I suggest stealing the "Character Encoding" icon from Firefox,
> as shown here:
> https://www.dropbox.com/s/scvpv0h14xdfs5v/Screenshot%202015-06-16%2010.01.39.
> png?dl=0
Thanks for the suggestion. I'd need an SVG since we changed the icons to SVG (bug 1150627). Do you have these icons as SVG. We're using this in TB:
https://dxr.mozilla.org/comm-central/source/mail/themes/shared/mail/icons/compose-toolbar.svg
https://mxr.mozilla.org/comm-central/source/mail/themes/shared/mail/icons/compose-toolbar.svg
(I don't even have a program to display the file, GIMP, IE, FF, Chrome, all don't show it, I will have to ask Richard Marti to help me).
I was actually thinking more along the lines of an icon that looks like a dictionary, just look for "dictionary+icon" in Google. What do you prefer?

OK, so before abandoning this bug ;-) I'll look into the following:
1) the "always-enabled" button, a click in the wrong context drops down the language menu. Although on second thought, that's not really that clean.
2) Two standard buttons, build-in styles, no funny business. Are you OK with a dictionary icon?

Comment 89

[Blake Winton (:bwinton) (:☕️)]

(In reply to Jorg K from comment #88)
> (In reply to Blake Winton (:bwinton) from comment #87)
> > > Have an independent "Language" buttons that doesn't try to amalgamate with
> > > the "Spelling" button. That's clear cut, single-language users who never
> > > change the language can simply remove it.
> > This does seem simpler and more understandable than the other options we're
> > considering.  I suggest stealing the "Character Encoding" icon from Firefox,
> > as shown here:
> > https://www.dropbox.com/s/scvpv0h14xdfs5v/Screenshot%202015-06-16%2010.01.39.
> > png?dl=0
> Thanks for the suggestion. I'd need an SVG since we changed the icons to SVG
> (bug 1150627). Do you have these icons as SVG. We're using this in TB:
> https://dxr.mozilla.org/comm-central/source/mail/themes/shared/mail/icons/
> compose-toolbar.svg
> https://mxr.mozilla.org/comm-central/source/mail/themes/shared/mail/icons/
> compose-toolbar.svg
> (I don't even have a program to display the file, GIMP, IE, FF, Chrome, all
> don't show it, I will have to ask Richard Marti to help me).
> I was actually thinking more along the lines of an icon that looks like a
> dictionary, just look for "dictionary+icon" in Google. What do you prefer?

I don't, no, but hopefully Richard will.  I asked Stephen (who created the Firefox version), but he doesn't have one.

> OK, so before abandoning this bug ;-) I'll look into the following:
> 1) the "always-enabled" button, a click in the wrong context drops down the
> language menu. Although on second thought, that's not really that clean.
> 2) Two standard buttons, build-in styles, no funny business. Are you OK with
> a dictionary icon?

Maybe…  Could you post the icon or a screenshot?  (In general, I'm happy with the idea. :)

Thanks!

Comment 90

[Blake Winton (:bwinton) (:☕️)]

Update: Stephen _did_ have the icon!  It's at http://f.cl.ly/items/130J3R1e3Y40420T1r3B/characterEncoding.svg  :D

Comment 91

[Jorg K (CEST = GMT+2)]

> > 1) the "always-enabled" button, a click in the wrong context drops down the
> > language menu. Although on second thought, that's not really that clean.
Just think of a single language user who clicks the button when he shouldn't and all he gets is a list of one language. Quite confusing, don't you think?

> > 2) Two standard buttons, build-in styles, no funny business. Are you OK with
> > a dictionary icon?
Thanks for the icon as SVG. I think it's not so intuitive to signify a language choice of dictionary.

I will post one here when I have one.

Richard:
We're thinking of splitting the "Spelling" button which also has the language selector in it into two buttons. The "Spelling" button will be enabled whilst in the message body, the language selector will always be enabled. See attachment 8622948 [details] for a mock-up. I'd lick to create an icon which looks like a little book. Just look for "dictionary+icon" in Google to get an idea. I need the icon as SVG and also PNG to support the old XP icons. Could you assist with designing and providing an icon in these formats? For XP we only need one icon, no hover or disabled version.

Comment 92

[Jorg K (CEST = GMT+2)]

Attachment: Dictionary icon

Something like:
http://findicons.com/search/dictionary
Could just be a simple "Aa".
Or better even:
https://icons8.com/tag/dictionary/
I grabbed one from the latter site and adapted it.

Comment 93

[Blake Winton (:bwinton) (:☕️)]

(In reply to Jorg K from comment #91)
> > > 1) the "always-enabled" button, a click in the wrong context drops down the
> > > language menu. Although on second thought, that's not really that clean.
> Just think of a single language user who clicks the button when he shouldn't
> and all he gets is a list of one language. Quite confusing, don't you think?

A little confusing, but easily-backed-out.  (We could spend time making it less confusing, but is that the best use of your time? ;) 

> > > 2) Two standard buttons, build-in styles, no funny business. Are you OK with
> > > a dictionary icon?
> Thanks for the icon as SVG. I think it's not so intuitive to signify a
> language choice of dictionary.
> Something like:
> http://findicons.com/search/dictionary
> Could just be a simple "Aa".
> Or better even:
> https://icons8.com/tag/dictionary/
> I grabbed one from the latter site and adapted it.

I like the image, but I worry a little about the license on icons we get from other places…  Can you post a link to their license, so that I can feel better about our using it?

Thanks,
Blake.

Comment 94

[Richard Marti (:Paenglab)]

Attachment: compose-toolbar.svg

I used the icon based on Blake's proposal of the FX icon (not used Stephen's SVG as it needed special treatment to look better with the border for Aero).

You can show it with list-style-image:

url("chrome://messenger/skin/messengercompose/compose-toolbar.svg#encoding");

and

list-style-image: url("chrome://messenger/skin/messengercompose/compose-toolbar.svg#encoding-inverted");

Comment 95

[Richard Marti (:Paenglab)]

Attachment: compose-toolbar.png

The normal toolbar for XP. I'm not really happy but I need to see it in composer to ponder it better.

Comment 96

[Richard Marti (:Paenglab)]

Attachment: compose-toolbar-small.png

The small toolbar for XP.

Comment 97

[Blake Winton (:bwinton) (:☕️)]

(I think the æ could be moved down a little to center it, maybe?)

Comment 98

[Richard Marti (:Paenglab)]

On FX it's also not centered. The XP ones are from FX PNG.

Comment 99

[Jorg K (CEST = GMT+2)]

Thanks!

So do we go with this ligature "æ" or a "dictionary" icon (with a doubtful license, I haven't looked yet)?

Comment 100

[Richard Marti (:Paenglab)]

I would say, use the icons I attached. If the XP icons don't fit we can try other in a new bug.

Comment 101

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons (v8)

Here we have two independent buttons with use the "æ" icon.
The part of making the button label show the current language will follow soon (I hope).

Comment 102

[Jorg K (CEST = GMT+2)]

Attachment: This is how the new button looks on Windows 7 (Aero).

Comment 103

[Blake Winton (:bwinton) (:☕️)]

(I'm not sure if your patch is a work-in-progress, but something's slightly messed-up in the icons-and-text mode on Mac.  https://www.dropbox.com/s/uvttt0l51wxtai8/Screenshot%202015-06-16%2022.41.38.png?dl=0  Does it work on Windows in the same mode?)

Comment 104

[Jorg K (CEST = GMT+2)]

Attachment: Windows 7 with icons and text aligned vertically

It's WIP only in so far as I plan to show the current language on the button instead of the label "Language". The rest should work with the patch I supplied, it works on Windows. Once again, the Mac begs to differ :-( The new button is straight forward. Windows Aero and Mac configurations are 100% the same. Unless of course the SVG provided by Richard makes the Mac cough.

Comment 105

[Jorg K (CEST = GMT+2)]

Note that the previous spelling button and attach button are type "menu-button", the new button is type "menu", so just a menu, not a clickable button. Perhaps this type generally doesn't work on Mac in vertical alignment. Did I say before ... and now the damn OS X comes along which does everything differently and tramples right over the otherwise quite feasible solution. :-((

Comment 106

[Richard Marti (:Paenglab)]

Attachment: 368915-new-button.patch (v9)

Jörg, messengercompose didn't have the support for button [type="menu"]. I've added it in your patch.

Blake, if you like can you test it on OS X?

Comment 107

[Jorg K (CEST = GMT+2)]

Richard: Thank you so much!! This stuff is terribly complicated for a novice like myself.

Comment 108

[Jorg K (CEST = GMT+2)]

Attachment: New look on Windows after applying Richard's patch

Richard, just something very pedantic. The arrow on the "menu" type sits one pixel below the arrow on the "menu-button" type. Is this intentional?

Comment 109

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 8623558 [details] [diff] [review]
368915-new-button.patch (v9)

Looks good on Mac! :)  My one final nit is that in the screenshot you posted in comment 104, Jorg, I believe the arrow should be beside the icon+text, not underneath it.  But ui-r=me with that fixed!

Comment 110

[Blake Winton (:bwinton) (:☕️)]

(Oh!  I see Richard's patch fixed the thing I mentioned.  Well, ui-r=me, then.  :)  If you can get the arrow moved up a pixel, that would be cool, too.)

Comment 111

[Jorg K (CEST = GMT+2)]

Blake, the screenshot from comment #104 is already outdated. In Windows it now looks like in comment #108 with the arrow to the side of icon and text.

Just to clarify: Do you want to approve it with the label "Language" or would you prefer to have a dynamic label that shows the language used? I was going to look into it hopefully today or tomorrow (depending on the research/effort involved).

Comment 112

[Jorg K (CEST = GMT+2)]

Wires crossed. Last question: Fixed label "Language" or dynamic label showing the language?

Comment 113

[Blake Winton (:bwinton) (:☕️)]

I like the dynamic label, if you can get it working.  But if you can't, using "Language" would be an acceptable fallback.

Comment 114

[Richard Marti (:Paenglab)]

Attachment: 368915-new-button.patch (v10)

This patch includes now a fix for the not aligned dropmarker. Not needed on XP. And while I checked this on XP I can say the language icon doesn't look strange together with the colored ones.

Comment 115

[Jorg K (CEST = GMT+2)]

(In reply to Blake Winton (:bwinton) from comment #113)
> I like the dynamic label, if you can get it working.  But if you can't,
> using "Language" would be an acceptable fallback.

Blake: I've been playing with the dynamic label. Surely we don't want to display the "en-US" and "es-ES" dictionary names, but instead the value of the menu item that's currently selected.
I have the following dictionaries installed and get the following menu entries:
English (US)
English (AU)
French (classic-reform)
French (classic)
French (reform)
French (modern)
German (DE)
Spanish (ES)
(the silly French dictionaries come as a set).
You can see the problem. The name of the language/dictionary can be very long making for a very wide button. If the language is changed, the width of the button and with it the width of the toolbar changes. Not really great.
I thought to truncate the label to the first space, but then the user can't see from the label, in which dictionary exactly there are in, since the label would only show "English", "French", etc.
Also there might be the off chance that a language name is two words, and discarding the string from the first space could be confusing.

Do you have an opinion?

Sorry that this is turning out to be a matter more complex than we thought.

Comment 116

[Jim Porter (:squib)]

Is a wide "Language" dropdown really any worse than a wide "Font" dropdown? If you only use one language and get grumpy at seeing "English (US)" all the time, you can just remove the dropdown from the toolbar.

Comment 117

[Jorg K (CEST = GMT+2)]

Attachment: with dynamic button label

Jim: You misunderstood, the button label will be wide, see picture.
Unless I truncate to the first space.

Comment 118

[Jim Porter (:squib)]

No, that's what I mean. I don't see an issue with that, just like I don't see an issue with the width of the "Font" selector in the formatting toolbar. There's plenty of space on the main toolbar.

Comment 119

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons, dynamic label (v11)

Here comes the (hopefully) final patch.
Dynamic button label, not truncated, so it can get rather wide for French users.

Comment 120

[Jorg K (CEST = GMT+2)]

Damn, there is a *var* languageToSet = Services.prefs.getCharPref("spellchecker.dictionary"); missing.

It works anyway, I'll fix this with any other nits that might arise.

Comment 121

[Jorg K (CEST = GMT+2)]

Damn again, there are problems during the customisation with the dynamic label if you drag the button off the toolbar into the "warehouse window", then change the language, then drag it back.
Also, there is no default label. I'll fix this now.

Comment 122

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons, dynamic label (v12)

Haha, the next "final" patch.

Comment 123

[Blake Winton (:bwinton) (:☕️)]

Squib: Did you want to take the UI-Review?  I'm generally happy with it, and don't have a strong opinion on the width of the button.

Comment 124

[Jim Porter (:squib)]

I doubt I'll have time to apply this patch in the next week or so because of Whistler, and I'd want to see this in action before I do a ui-review. My main concern is, "What happens to the width of the button when you change languages?" I think it should use the maximum width of any of the available languages, like the Font selector does. In general, we should avoid making the main toolbar buttons change position/size, since that can be a little confusing.

I suppose another option would be to specify a maximum width that the button can never exceed, and truncate the text if it's too long. Then just add a tooltip with the full name so people can still see it.

Comment 125

[Jorg K (CEST = GMT+2)]

(In reply to Jim Porter (:squib) from comment #124)
> My
> main concern is, "What happens to the width of the button when you change
> languages?" I think it should use the maximum width of any of the available
> languages, like the Font selector does. In general, we should avoid making
> the main toolbar buttons change position/size, since that can be a little
> confusing.
> 
> I suppose another option would be to specify a maximum width that the button
> can never exceed, and truncate the text if it's too long. Then just add a
> tooltip with the full name so people can still see it.

I think both these options are not very good.
In option 1) you waste a whole lot of space if there is a language with a long name, like "French (classic-reform)". This new button will then determine the look of the whole toolbar. Before the language switching was hidden in a little down arrow within the spelling button.
In option 2) you might cut right through the text, which will look extremely ugly.

My suggestion is to truncate the display from the first space, or better to remove the stuff from the opening parenthesis to the right and the space in front of it. At least in English you will get short language names of a length that doesn't vary much:
English
Spanish
French
German
Italian
Greek.

I you want to see this, I can modify my patch to do this.

Comment 126

[Thomas D. (:thomas8)]

Just my 2 cents, but if, as bwinton says, we need to chose between pest and cholera, I'd prefer pest ;)

I'm a bit worried that we're starting to design around our own design shortcomings, which makes this a case of ux-implementation-level: "interfaces should not be organized around the underlying implementation and technology in ways that are illogical, or require the user to have access to additional information that is not found in the interface itself."

In a nutshell:
-> Unpacking the dual button (spell-dialog + language-select) into two separate buttons is not very useful, more clutter than benefit (see details below)
-> Considering all unfortunate circumstances, I like the first solution proposed by Blake (bwinton) in his comment 82:
- enable the dual button for subject
- in subject, make clicking the main part of the button (spell-dialog) do nothing

Here's what I see:

We have 3 functions
F1) inline spell-check (good for quick fix, bad for long texts and systematic, efficient spell-check as you need to click on each error manually, practically not keyboard-accessible)
F2) dialog spell-check (too bulky for quick fix, but good for long texts and systematic, efficient, post-fact spell-check)
F3) language selection (currently realized in a very basic manner where you can only choose single language for the entire body and subject)

F1 (inline) seems to work as it should, everywhere.
F2 (dialog) works as it should for body; not available for subject at all (that's a bug which is not yet explicit in this bug; ux-consistency, ux-efficiency, accessability); spell-button disabled in subject; using the dialog for body will *not* iterate through subject.
F3 (language) works as it should for body; not available when in subject at all; spell-button including language dropdown disabled in subject (this bug).

Thoughts:

1) We don't know how important or frequent it is for users to use F2 (spell-dialog); but it's a powerful tool that we currently offer.
2) It's a bug which should be fixed in the long run that F2 (spell-dialog) is not available in subject (but we're not sure how hard it is to fix that).
3) It's a bug which should be fixed that F3 (language select) is not available in subject (and we're here to fix that).
4) It's very basic (read poor) design that we only allow one language for entire body+subject which should also be fixed in the long run (however, it's unlikely to happen unless editor does that for us).

Assuming that 4) (having different sections with different language) is really beyond us right now:

5) If there's only one language for body+subject, the maximum number of times to change that for the same composition will be one time, when initial language setting was wrong. With wrong language, whole text will be red. With right language, user will no longer be interested in knowing the language, and will not change it again. So as long as we don't offer multi-language for different sections of text, it's quite uninteresting to show the currently selected language. If it's not interesting, it's distracting if displayed on xxl button labels, or attracting attention by truncation and ugly half-readable labels, so probably violating ux-minimalism. Given 4), the usefulness of a dedicated language button is currently low, and the current dual-button design reflects that really well: big click target for spell-dialogue (as a powerful tool); small dropdown target for language switch (maximally used once per composition, then irrelevant).

7) However, as reporter and Jorg correctly point out, that one time of wanting to change the language might very well occur when user starts off with writing the subject (this bug).

8) Having two separate buttons will force all users of F2 (spell dialog) who also need F3 to change their language (maximally once per composition) to keep *two* bulky buttons without any practical benefit over the current dual button (F2+F3) IF only it was enabled for subject...

9) As long as we don't fix bug of F2 (no spell-dialog for subject), doing nothing from enabled button is arguably not much worse than disabling the button altogether; in both cases, we don't offer spell-dialog for subject, which is a bug which users will notice, and which we need to fix in the medium or long term. In both cases, we are violating ux-principles. However, using the full-blown spell dialogue for subject might actually be less likely/frequent than for body, while changing the language when writing subject seems more likely/frequent compared to that.

CONCLUSION
-> Unpacking the dual button (spell-dialog + language-select) into two separate buttons is not very useful, more clutter than benefit (see details above)
-> Considering all unfortunate circumstances, I like the first solution proposed by Blake (bwinton) in his comment 82:
- enable the dual button for subject
- in subject, make clicking the main part of the button (spell-dialog) do nothing

Comment 127

[Thomas D. (:thomas8)]

Note that both of bug 491055, bug 408207 have had all but zero interest, me-too comments, or more comprehensive UX analysis weighing pros and cons.

Comment 128

[Thomas D. (:thomas8)]

Put in another way, although having a dedicated language selector may not be a bad idea as such, but I'm not sure if it's wise to realise that on the main composition toolbar, because it seems to manifest the wrong design that we can only chose a single language for all of subject + body. Instead (in a 21st century global world), language should become a text property like font or font size, thus probably more aptly placed on the formatting toolbar, to set different languages for different parts of the text.

Also, given the near-zero number of complaints on this, I'm not sure why we should remove and split up the current dual button when just enabling it might do the trick (notwithstanding the need to fix other bugs in this corner).

FTR:
- Office 2013 seems to get away with a language button (not showing the currently active language for each section of text).
- Older versions of Office showed the language setting of text under the cursor in the status bar, double-click on status bar allowed changing the language for selection.

Comment 129

[Thomas D. (:thomas8)]

Also wrt to long language labels, that relates to bug 728775...
We actually manage to refer to the same language in as many as 3 different ways:

English (US) - Spell-button dropdown
English (United States) - Inline spellcheck, context menu > Languages
English/United States - Main Check Spelling dialog, dictionaries list

Fortunately English does not have classic-reformed variants like French or German...

Comment 130

[Jorg K (CEST = GMT+2)]

Thomas:
You're a little late for your two cents of wisdom after four people (myself, Richard, Blake and Jim) invested countless hours into the solution which has already received ui-r+ in comment #109 when the button label was still fixed. We're now discussing the dynamic of the label.

Thanks for mentioning bug 491055:
... bigger click target for language switching, show current language
and bug 408207:
Check spelling button should display selected dictionary language

Obviously the proposed solution of two independent buttons would fix these two bugs, since a) the button *is* a larger click target and b) will show the selected language/dictionary.

But let's step back for a second and look at the current "spelling+dictionary" button. I would claim, that two functions have been merged into the one UI element which don't really below together. Why should execution and options be merged into one? Take an automatic-gear car. The accelerator and the gear selector are two different controls, although in most cases the driver will use the gear selector only twice in the whole trip: To move from P to D and back to P.

If you look at other e-mail packages, the dictionary choice is buried deeply in some options, it's not available in the window you compose your e-mail in. TB has brought this option to the fore, but glued it to a spot where it doesn't really belong.

Let me therefore summarise the advantages of the two button solution:
- clear and simple
- large click target
- additional information
- fully customisable: Those who only have one language can get rid of the language button,
  those who don't use full spell-check can get rid of the spelling button.
- not violating any UX principles.

Personally I've never used the attach or save button, so I'm not worried that we will have another button that is not useful to all users.

Comment 131

[Jorg K (CEST = GMT+2)]

Attachment: This is how it looks with the shortened language

(In reply to Thomas D. from comment #129)
> 1) English (US) - Spell-button dropdown
> 2) English (United States) - Inline spellcheck, context menu > Languages
> 3) English/United States - Main Check Spelling dialog, dictionaries list [and also in Tools > Options]

This is rather unfortunate, indeed. As was pointed out, bug 728775 provides a patch to unify items 1) and 2), which has been hanging around since 2012 (!!). I'm happy to look into it, once this bug here has landed.

In view of unifying items 1) and 2) I repeat my suggestion to remove the "(local flavour)" from the button label, so that button label says "English" and the dropdown, once fixed, would say "English (United States)" just like the right-click menu.

I don't want to submit yet another patch, whoever wants to try needs to change this
  document.getElementById("spellingLanguage").label = item.getAttribute("label");
to this:
  document.getElementById("spellingLanguage").label = item.getAttribute("label").replace(/ \(.*/, "");
in /mail/components/compose/content/MsgComposeCommands.js

Comment 132

[Magnus Melin [:mkmelin]]

Comment on attachment 8623881 [details] [diff] [review]
Proposed patch to have two independent buttons, dynamic label (v12)

Review of attachment 8623881 [details] [diff] [review]:
-----------------------------------------------------------------

(path has bitrotted)

I don't think it's a good idea to show the current language as the button text. It feels malplaced. By comparison, we don't show "Encrypted message" if you choose that in the Security button either.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +3121,5 @@
>  
> +function updateLanguageButtonLabel(language)
> +{
> +  InitLanguageMenu();
> +  let languageMenuList = document.getElementById('languageMenuList');

nit: prefer double quotes (it's not nice when you have a mix for no reason)

@@ +3155,5 @@
>      // now check the document over again with the new dictionary
>      if (gSpellChecker.enabled)
>        gSpellChecker.mInlineSpellChecker.spellCheckRange(null);
> +
> +    updateLanguageButtonLabel (languageToSet);

nit: no space before (

::: mail/locales/en-US/chrome/messenger/messengercompose/messengercompose.dtd
@@ +215,5 @@
>  <!ENTITY quoteButton.tooltip "Quote the previous message">
>  <!ENTITY addressButton.tooltip "Select a recipient from an Address Book">
>  <!ENTITY attachButton.tooltip "Include an attachment">
>  <!ENTITY spellingButton.tooltip "Check spelling of selection or entire message">
> +<!ENTITY languageButton.tooltip "Change spelling language">

Change spell checking language

Comment 133

[Jorg K (CEST = GMT+2)]

(In reply to Magnus Melin from comment #132)
> (patch has bitrotted)
Due to bug 1175908? Anyway I'll fix that together with the other nits if we ever get through the ui review ;-(

I'd personally love to see the language displayed. Obviously other people would too (bug 491055, bug 408207). We could of course have yet another preference ;-)

Comment 134

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons, dynamic label (v13)

1) fixed rot from bug 1175908
2) addressed nits from comment #132
3) added shortening of language name as per comment #131.

Comment 135

[Thomas D. (:thomas8)]

Hi Jorg,

1st of all, I'm more than happy that you're active in coding, and I appreciate the energy :)
2nd, I appreciate that you've invested a lot of time and efforts into this and you want to land your baby...
3rd, UX is a delicate business, involving a multitude of different users and use patterns, scenarios, principles, scopes of argumentation, assumptions, subjective preferences, experience, time factors, compromise... which are often conflicting, not easy to navigate and balance out. You'll note how experienced UX people like Blake are very careful in phrasing their UX contributions, in awareness of the tentative nature of ANY UX design...

(In reply to Magnus Melin from comment #132)
> Comment on attachment 8623881 [details] [diff] [review]
> I don't think it's a good idea to show the current language as the button
> text. It feels malplaced. By comparison, we don't show "Encrypted message"
> if you choose that in the Security button either.

+1. I agree with Magnus that just "Language" as a general label for the language selector will be fine. It's tempting and seemingly helpful at first sight to show the specific name of the language, but at second thoughts, what's the actual benefit in the current design?

* we only offer 1 language for the entire message, so after setting the "right" language maximally once per composition, that language label will never change and will have very little or no informational value after that. Furthermore, with Jorg's plan to truncate after first word, potentially interesting aspects are actually cut out (am I using traditional French, or classic-reformed?); bet we can't show them in full either because of button size.
* If it's not very informative, it might be unnecessarily distracting (against ux-minimalism); more so because it's hard to predict how long even that single-word language label will be in different localisations, compare:
- French        (EN locale)
- Französisch   (DE locale) (that's almost twice as long)
* For users who want to change the language (once per composition), can we safely assume that specific "Name of language" label is easier to find than general "Language" label? Will they always know which "wrong" language is currently set? As Magnus points out, the current UI seems to prefer the general names of settings or actions, as opposed to showing specific values of the setting. Perhaps that's because in the long run, *especially* for those who actually use the feature, it'll be easier to remember just the single general term, as opposed to remembering the current wrong language from an unlimited number of languages before changing that. I'd think it's slightly less intuitive to look for a label saying "English" when I actually want "German" or "Chinese" than to just look for the general label to change "Language".
* "Dynamic label"? I don't think the size of the button should change depending on the language I choose, because that will change the position of all subsequent buttons which is odd and unbecoming for muscle-memory... (against ux-habit-formation ;) So we'd need a fixed size for the language label button, back to truncation...

So here are my tentative thoughts/proposals:

1) Keep and enable dual "Spelling" button as default (this bug)
Per summary of this bug, we're here because the current "Spelling" dual button is wrongly disabled in subject, which has two consequences:
a) spelling language can't be changed (with focus in subject; this bug).
b) spell check dialogue can't be used for subject (at all; needs a new bug).
The immediate solution which springs to mind is to just enable the button, as proposed in Blake's comment 82, which will will solve a) and arguably not make things worse for b).
We're not here to remove the dual button (and I haven't seen any requests for that). For users of the spell-check dialogue who occasionally want change or check their language, the dual button is just right.

2) OK to add optional "Language" button and non-menu "Spelling" button to customization palette of composition toolbar, but don't make them default 
- Other types of users who never use the spell-check dialogue, or users who change their language more often, might prefer a separate, dedicated "Language" selector button as a bigger and more explicit click target. Again, bugs are not showing much interest in that, perhaps 2 or 3 individuals, no votes, almost no comments for years.
- I have some reservations about adding that "Language" button to the main composition toolbar because in the long run, when editor allows having sections with different languages in one composition, it will no longer be a global property of composition, but more like a text formatting which should be on formatting toolbar. In that case(!), it will also make sense to display the actual language.
- But I'm ok with adding a "Language" button to toolbar customization palette of composition toolbar for now.
- If we add a "Language" button, it makes sense to offer a non-menu flavor of the "Spell" button from toolbar palette, too (without the language dropdown indicator). Otherwise the existing dual Spell button might still be sufficient/acceptable.
- I'm against making two separate "Spelling" AND "Language" buttons the default for everyone; many users might use neither of the two, and that's wasting an awful lot of space and creating permanent visual distraction for something which is actually rarely used in the composition process.
- I'm against making only "Language" button the default for everyone; chances are that the same power or professional users who switch languages often enough to need the "Language" button will also use the advanced "Spelling" dialogue. I think inline spell-check and spell-check dialog are two different functions in their own right and we have no actual qualitative or quantitative data on their usage; there are common tasks like "replace all", "ignore only this one" which are not even possible using inline spell-check. I also think tucking "advanced" functionality too far away, while good for ux-minimalism, is bad for ux-discovery. If somebody thinks that spell-check dialogue is no longer important enough to have its own button, I think the right place for that would be a dedicated bug with a careful discussion of pros and cons. (We could consider reversing the logic and having a "Language" button with a "Check spelling..." menu for the dialogue at the top of the dropdown menu; and/or offering access to the spell-check dialogue from msg body context menu).

3) Design of dedicated "Language" button: [Language v] (general label, fixed size)
- As laid out above following up on Magnus' point, under the current design there's not much benefit of showing the language all the time when you can change it maximally once per composition and it will be the same everafter.
- As laid out above, button size should not change and move other buttons around when using the button

Comment 136

[Jorg K (CEST = GMT+2)]

Jim:
Can you please decide what we're going to do:
1) Two buttons, fixed label, attachment 8623558 [details] [diff] [review], ui-r+ by Blake. Magnus' preference.
2) Two buttons, variable label (also fixes bug 491055 and bug 408207).
3) Some other solution (that should be clearly defined *before* coding starts).

I should have stopped after the ui-r+, but I continued with the dynamic label, since Blake said in comment #113:
> I like the dynamic label, if you can get it working.
> But if you can't, using "Language" would be an acceptable fallback.

Let's not add more patches and comments to the bug, let's take a decision!

Comment 137

[Thomas D. (:thomas8)]

(In reply to Jorg K from comment #136)
> Jim:
> Can you please decide what we're going to do:
> 1) Two buttons, fixed label, attachment 8623558 [details] [diff] [review]
> [review], ui-r+ by Blake. Magnus' preference.
> 2) Two buttons, variable label (also fixes bug 491055 and bug 408207).
> 3) Some other solution (that should be clearly defined *before* coding
> starts).

Jorg, that's omitting some of the key points (deliberately?), apart from ignoring what I've said.

1) This bug requests something very simple: Enable dual "Spelling" button (combined spell-check + language selector) which is wrongly disabled in subject (as suggested by bwinton in comment 82).

2) You are trying to implement a new, separate language selector button (somewhat beyond the scope of this bug) - fair enough, some users who need to change their language will find this useful.

From there, there are new questions some of which have not been adequately addressed yet.

3) Should the "Language" button become a default button on the toolbar, or should it just be optional, available via toolbar customization? I.e., do we think there are enough users out there who are regularly changing their composition language to justify that button taking up space on the main composition toolbar, worse if it's actually TWO buttons, Spelling AND Language (which are currently nicely combined into one, and we do not have any significant complaints about that)?

4) So what happens to the current dual button?

> I should have stopped after the ui-r+, but I continued with the dynamic
> label, since Blake said in comment #113:
> > I like the dynamic label, if you can get it working.
> > But if you can't, using "Language" would be an acceptable fallback.
> 
> Let's not add more patches and comments to the bug, let's take a decision!

I've laid out my detailed proposal in comment 135 and would want that to be considered in the decision taking process:

* Keep and enable dual "Spelling" button as default (combined Spell-check + Language selector; this bug)
* OK to add optional "Language" button and non-menu "Spelling" button to customization palette of composition toolbar, but don't make them default
* Design of dedicated "Language" button: [Language v] (general label, fixed size)

Comment 138

[Richard Marti (:Paenglab)]

Comment on attachment 8625174 [details] [diff] [review]
Proposed patch to have two independent buttons, dynamic label (v13)

Taking Jim's ui-r as he has no time.

I think showing the selected language is a win. The additional language info in the brackets isn't needed here.

ui-r=me

Comment 139

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons, dynamic label (v14)

Carrying forward Richard's ui-r+.

Changed the way the language button is updated. It's now updated using an observer. This makes it more robust when someone else changes the "lang" attribute, for example in the patch in attachment 8626813 [details] [diff] [review] in bug 1176671.

Comment 140

[Magnus Melin [:mkmelin]]

Still thinking about what to do with this bug. 
I'm thinking the language button should probably go into the "formatting toolbar" instead.

Comment 141

[Richard Marti (:Paenglab)]

But in text only mode where is no formatting toolbar.

Comment 142

[Jorg K (CEST = GMT+2)]

Attachment: Toolbar Softmaker style: with colourful XP icons brought back to WIn7

Hmm, we have ui-r+ by Blake for the version without the dynamic label and ui+r by Richard for the version with the dynamic label. It even looks good with the old icons. For people who write in multiple languages it's a bonus to see which language they're in.

Comment 143

[Jorg K (CEST = GMT+2)]

If we implement a separate button for the language selection (as ui-approved twice), the only logical place is next to the spelling button on the compose toolbar for two reasons:

It can't go onto the formatting toolbar since there is no formatting toolbar for plain text messages (comment #141).

It's not logical to place the button below the subject field, since the language selection affects the subject field (which is actually how the whole thing came about).

If anything, the spelling button is placed on the wrong menu, since it's context-sensitive just as the buttons on the formatting menu (I can just hear Thomas D. saying: "That's why it should be made non context-sensitive"). But we only have two menus, not three (compose/general, spelling/language, formatting), so the spelling/language needs to go onto the compose menu.

If (ever) in the course of time the underlying editor were to support attaching a language attribute to a paragraph of text, then we would have to rethink the user interface. At that point the language would become a formatting attribute, only available in HTML messages. (But shock horror: Even then we'd need an overall language selector for the subject and paragraphs without a special language attribute).

I think it's safe to say that this won't happen any time soon, so the ui-approved new button next to the existing spelling button is the way to go for now.

Comment 144

[Christian Hudon]

I filed another bug a while ago about the poor usability of the spellcheck button for multi-lingual users.
I switch often between French and English (and others), depending on who am I writing to. Basically, from my perspective as a multi-lingual user, the current spellcheck button has two problems: 1) it doesn't show what the current language is; and 2) the click target for the drop-down to switch language is very narrow, making this a slow operation to do. In contrast, the target for the "spell check" button, which I never use now that "spellcheck as you type" exists, is much larger. The proposed design here would be a great solution for me and for these problems. Please consider implementing it.

And oh yeah, in practice for me (and all the multi-lingual users I know), writing different paragraphs in different language is just not something I do (or see done), except maybe for text translation (i.e. very, very rarely, if at all). The language I use depends on what the recipient(s) of the email understand. If I'm mixing multiple language, it will be because the recipient and I share more than one language in common. What will happen then is that most of the email will be in one language, but sometimes a few words or an expression will be in other languages. (Example: French email, with some computer-related terms in English.) And no, I won't bother to select the words in the other language and switch their language property to the right language. At that level, it's a waste of time and simply not worth the effort, in my opinion. So, the proposed design is a great fit for this. I don't think the "paragraphs in different languages" use case is a very important, and not worth delaying a solution to what is a much, much more common problem for multi-lingual users.

And actually, if the solution proposed here is implemented, I would personally keep the "switch language" button and remove the "spell check" one. And I don't think I'm the only one. From what I see around me, people don't use a "spell check" dialog box anymore now that "spell check as you type" (aka the red squiggly underlines) exists.

Comment 145

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons, dynamic label (v14, refreshed)

Periodic refresh due to bit rot.

Carrying forward Blake's (comment #109) and Richard's (comment #138) ui-review+

Comment 146

[Magnus Melin [:mkmelin]]

Comment on attachment 8646001 [details] [diff] [review]
Proposed patch to have two independent buttons, dynamic label (v14, refreshed)

Review of attachment 8646001 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, so let's get this bug resolved.
Let's go ahead with the extra button, but please have the button label be a static "Language" and not change to the selected language, like I mention somewhere before. 

A bunch of the comments below are probably irrelevant after that. But anyway, I don't think I need to see this patch again so if you make the changes, feel free to carry the review forwards.
And again, sorry for the long delay.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +2111,5 @@
> +  // Observe the language attribute so we can update the language button label.
> +  gLanguageObserver = new MutationObserver(function(mutations) {
> +    mutations.forEach(function(mutation) {
> +      if (mutation.type == "attributes") {
> +        if (mutation.attributeName == "lang") {

i'd combine these if clauses

@@ +2115,5 @@
> +        if (mutation.attributeName == "lang") {
> +          updateLanguageButtonLabel();
> +        }
> +      }
> +    });    

nit: trailing whitespace

@@ +3178,5 @@
>  
> +function updateLanguageButtonLabel()
> +{
> +  InitLanguageMenu();
> +  let language = document.documentElement.getAttribute("lang");

move this definition down to close where it's used, after "let item"
(or well, don't need this method I suppose)

Comment 147

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons, dynamic label (v15, refreshed again, CSS un-bitrotted)

Thank you for the review. I've addressed the nits.

So far, I haven't removed the dynamic label, since this is one of the *features* of the solution, see ...
Comment #113 by Blake:
"I like the dynamic label, if you can get it working."
Comment #138 by Richard:
"I think showing the selected language is a win.".
Comment #144:
"the current spellcheck button has two problems: 1) it doesn't show what the current language is;"

Can we convince you?

Richard, can please review the CSS again. There has been so much movement in the area and I had to refresh the patch so many times, that I'm now unsure that I got it right.

There also used to be this in the Windows aero file, which also isn't required any more, right?
+.toolbarbutton-menu-dropmarker {
+  margin-top: 0;
+}

Comment 148

[Jorg K (CEST = GMT+2)]

> There also used to be this in the Windows aero file, which also isn't
> required any more, right?
> +.toolbarbutton-menu-dropmarker {
> +  margin-top: 0;
> +}
Apparently moved to mail/themes/windows/mail/messenger.css

Comment 149

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K from comment #147)
> Can we convince you?

If you insist, I could create a preference to switch the dynamic off.

Comment 150

[marco]

Sorry to be naive / noobish, but may I ask something since I really like what I see in the screenshots: When will this be in Thunderbird? Or will there be only a patch for insiders here? In this case, how do I use it? I only see a big list of code...

Comment 151

[Jorg K (CEST = GMT+2)]

(In reply to marco from comment #150)
> When will this be in Thunderbird?
We're planning on landing this on TB 43 in the next few days (fingers crossed). Once landed, it will be in the Daily version, then moving to Earlybird (alpha), then to beta and next year in March it will come out in the ESR 45 release. See details here, Thunderbird has approximately the same schedule.

Comment 152

[Richard Marti (:Paenglab)]

Comment on attachment 8647727 [details] [diff] [review]
Proposed patch to have two independent buttons, dynamic label (v15, refreshed again, CSS un-bitrotted)

Review of attachment 8647727 [details] [diff] [review]:
-----------------------------------------------------------------

Only checked the patch on Windows and it looked good. The other CSS code is also correct.

Comment 153

[marco]

(In reply to Jorg K from comment #151)
> next year in March it will come out in the ESR 45 release. 

Really, it will take until March 2016? Wow, didn´t think TB development would be so slow. Don´t take this as an offence please, I am a total outsider and just got here with some ambition to find out about things like this. If I can have that faster, I would be glad, isn´t there an add-on that is doing this? Could you just develop an add-on for the time being, would that be available earlier?

Comment 154

[Magnus Melin [:mkmelin]]

Well if you want it sooner you could always start using nigtly builds, betas or alphas. 
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-comm-central/

Comment 155

[Jorg K (CEST = GMT+2)]

Hey, you didn't answer the question from comment #147:

So far, I haven't removed the dynamic label, since this is one of the *features* of the solution, see ...
Comment #113 by Blake:
"I like the dynamic label, if you can get it working."
Comment #138 by Richard:
"I think showing the selected language is a win.".
Comment #144:
"the current spellcheck button has two problems: 1) it doesn't show what the current language is;"

Can we convince you?

And from comment #149:
If you insist, I can create a preference to switch the dynamic off.

Comment 156

[Magnus Melin [:mkmelin]]

(In reply to Jorg K from comment #149)
> (In reply to Jorg K from comment #147)
> > Can we convince you?

Unfortunately, I don't think so, as I think it'd be very inconsistent. The Save button would show "Draft", the Security button would show "Signed and Encrypted" in comparison. They aren't, for obvious reasons.

Alternative behaviors for things like this should be left to add-ons. Every pref has a way higher maintenance cost in the long run than people realize.

Comment 157

[Magnus Melin [:mkmelin]]

Not that it's a vote, but Thomas also was in favour of the static button label, comment 135.

Comment 158

[Jorg K (CEST = GMT+2)]

So how do we settle this?

The UX people Richard and Blake (who according to https://wiki.mozilla.org/Thunderbird:UX/Team have a say in this) have approved a dynamic label.

Not that's a vote, but I have at least five people in favour (Blake, Richard, myself, comment #144 and comment #153) and two against.

Comment 159

[marco]

So may I also raise my hand for the dynamic button...? :-) 

It would be great to always see at a glance what language is currently set. 

And I don´t think this "inconsistancy" is too bad. Because it´s completely obvious what the button is for if it´s showing the language name instead of "language", while a button "draft" is not obviously a save button with just a "draft" setting. It´s different. 
And if I understood it right, there will be two buttons, one for the normal spelling options and one for the language (which is bigger than the small arrow down button that we have now, so that we also can hit it better). So this second button is more like a "paired" button to the spelling button anyway, so even more clear what it is anyways. Could be even designed a little like this. 


BUT even better would be a language auto-detect anyway ;-)))) (then still an indicator field showing the detected language would be good to have, so let´s do the first step to be prepared for the future ;-))

Comment 160

[Jorg K (CEST = GMT+2)]

Yes, you can raise your hand ;-)

I agree, the comparison of the language button showing the currently selected language with the "Save" and "Security" button is misleading. The language setting determines which dictionary is used for the inline spell check in subject and message body, and it would be good to see the current setting immediately (as was also requested in bug 491055).

As for the language autodetect: Try this add-on:
https://addons.mozilla.org/en-us/thunderbird/addon/automatic-dictionary-switching

Comment 161

[Christian Hudon]

I also think the dynamic language button would be much more useful. And I don't really understand the consistency comment. In my mind, this is about supporting users' common workflows. So for the language button, here are the possible workflows for multilingual users (i.e. people who need to switch between spellcheck languages more than once in a blue moon) when composing an email. Let's say this is me, who sends about half of my emails in French and the other half in English.

For the static button (also same workflow for current released version of Thunderbird):

First alternative:

1. I click on the compose button to open the compose window
2. I fill in the destination and subjects fields
3. I start writing my email body
4. Half the time, I see red spellcheck squiggles under all the words, because the spellcheck is set to the other language. I break the flow of my writing to...
5. 50% switch the spellcheck language to the right one and
6. 50% resume my writing.

Second alternative:

1. I click on the compose button to open the compose window
2. I fill in the destination and subjects fields
3. Remembering the annoyance above, I click on the static button to check that the language is the right one
4. Half the time, the language was the right one, and that button click to be able to check that delayed me needlessly from starting to write my email (which is what interests me).
4. The other half, I switch the spellcheck language to the right one.
5. I start writing my email body

Both choices are annoying. Compare to the workflow for the dynamic button:

1. I click on the compose button to open the compose window
2. I fill in the destination and subjects fields
3. Half the time, I see that the language is not set to the right one, so I switch it
3. The other half, I move right to writing the body of my email.

Advantages: I click on the language switch button only when needed (wrong language is set) instead of clicking on it all the time or having my writing's flow interrupted half the time. And no annoying choice between two unsatisfying alternatives. That's why the dynamic language button is better for me as a multilingual user. In contrast, I really don't see how changing the "save" button to "draft" would make anyone's common workflow faster or better. (But on the other hand, having the encryption button display the current state would also help save people time by saving them the trouble of clicking every time to check that the encryption for the email is in the state they want. But that's for another bug report.)

So, please include the patch with the dynamic button. I've been waiting for this for at least 10 years as a multilingual user, and I've probably wasted hours of my life clicking on that button when the language was the right one because it didn't show what the current language setting for spellcheck was.

Comment 162

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch to have two independent buttons, static label, status display (v16)

Language display in status bar.

Comment 163

[Thomas D. (:thomas8)]

(In reply to Jorg K from comment #162)
> Created attachment 8648326 [details] [diff] [review]
> Proposed patch to have two independent buttons, static label, status display
> (v16)
> 
> Language display in status bar.

Could you please attach a screenshot of that?

Comment 164

[Jorg K (CEST = GMT+2)]

Attachment: Screenshot of language display in status bar

Will only be displayed if more than one dictionary is installed.
Full language name is shown.

Comment 165

[Richard Marti (:Paenglab)]

Comment on attachment 8648326 [details] [diff] [review]
Proposed patch to have two independent buttons, static label, status display (v16)

I'm not a fan of this but I can life with this. I like the dynamic button more. What about when the status bar is hidden? Then the selected dictionary isn't shown.

Please can you move the dictionary text to the end of the status bar? It's not really a status feedback like saving the message. And when you save the message the dictionary text is overwritten and then no more shown.

And maybe we could always show the dictionary text also when the button is removed from the toolbar.

Comment 166

[Richard Marti (:Paenglab)]

(In reply to Jorg K from comment #164)
> Created attachment 8648350 [details]
> Screenshot of language display in status bar
> 
> Will only be displayed if more than one dictionary is installed.
> Full language name is shown.

It was here also shown with only en-us installed.

Comment 167

[Jorg K (CEST = GMT+2)]

(In reply to Richard Marti (:Paenglab) from comment #165)
Thanks for looking at it.

> I'm not a fan of this but I can life with this. I like the dynamic button
> more. What about when the status bar is hidden? Then the selected dictionary
> isn't shown.
I'm not a great fan of this either, also not from a work-flow perspective. The control is up above and the feedback is down below.
What can I do of Magnus reserves the right to overrule two UI approvals?
I offered a preference, but that was also rejected.

> Please can you move the dictionary text to the end of the status bar?
Hmm, to the end, you mean the right. Not really without drilling it open a whole lot more.
At the end, we show the encryption status, so a little envelop with a red seal.

> It's not really a status feedback like saving the message. And when you
> save the message the dictionary text is overwritten and then no more shown.
Indeed. I didn't notice that. I'd have to fix that.

> And maybe we could always show the dictionary text also when the button is
> removed from the toolbar.
That is the case already.

> It was here also shown with only en-us installed.
I noticed that now on a different profile. I'd have to look into it.

Comment 168

[Jorg K (CEST = GMT+2)]

Comment on attachment 8648350 [details]
Screenshot of language display in status bar

Semi-good idea, but it didn't fly.

Comment 169

[Jorg K (CEST = GMT+2)]

Comment on attachment 8648326 [details] [diff] [review]
Proposed patch to have two independent buttons, static label, status display (v16)

Too much trouble that it's worth.

Comment 170

[Jorg K (CEST = GMT+2)]

I suggest to land attachment 8647727 [details] [diff] [review] as it is with the dynamic label. I am not offering any other option (other than being willing to add a preference to switch the dynamic label off (not on!)).
This had ui-review+ and review+ for the technical side.
Too bad Magnus doesn't like it and can't be convinced.

I think we have a few users who will make good use of the feature, there is also a very good use case in comment #144 and comment #161.

This would come out in TB 45 together with the correspondents column, so users would see that TB is being actively developed and is modernising the user interface to include useful features. Single-language users can just remove the button, since they know which language the always use.

So I repeat the question from comment #158: How do we settle this?

Comment 171

[Thomas D. (:thomas8)]

(In reply to Jorg K from comment #170)
> I suggest to land attachment 8647727 [details] [diff] [review] as it is
> with the dynamic label. I am not offering any other option (other than being
> willing to add a preference to switch the dynamic label off (not on!)).

I think it's less about what you're willing to offer and more about finding the best possible solution after weighting pros and cons in terms of ux principles.

> This had ui-review+ and review+ for the technical side.
> Too bad Magnus doesn't like it and can't be convinced.

I'll assume that you mean "unfortunately" because "too bad" would have the wrong tone.
Magnus has put forward a valid argument of ux-consistency, which is less about /liking/ but more about /weighting/ different ux factors. And he is not the only one who is sceptical. In support of his scepticism, my comment 135 has a detailed UX analysis to which you've hardly responded.

> I think we have a few users who will make good use of the feature, there is
> also a very good use case in comment #144 and comment #161.

Yes, there's a minimal ux-efficiency advantage for the subset of multilingual users, and only for that scenario where you set the language /before/ writing the body text (after writing body text, red markers all over are a clear indication of wrong language, and it's 2 clicks to change that both before and after the patch, and "Language" imo is arguably still slightly easier to find than an ever-changing label of the /wrong/ language I'm /not/ writing in).

However, I really wonder how many of our users are regularly changing their spelling language? 5%? 10%? From the total number of our users, I'd think that the vast majority works in primarily monolingual settings where they might change their language perhaps 5 times per year for Christmas greetings or some foreign correspondance. "A few users" is not enough to force this as a default onto everyone...

> This would come out in TB 45 together with the correspondents column, so
> users would see that TB is being actively developed and is modernising the
> user interface to include useful features. Single-language users can just
> remove the button, since they know which language the always use.

I assume that a large majority of our users will change their language rarely, if ever. For that assumed majority, status quo (combined button) is just fine (if it would work in subject, this bug), and the extra language button always showing the same language is just clutter. Just removing the language button is not a good solution for them because then it'll be much less intuitive than now to change or check the language IF they want to change it.

> So I repeat the question from comment #158: How do we settle this?

Responding to proposals would be a good start. I have proposed before that we can realize the best of both worlds (Jorg's patch and some well-founded scepticism concerning the majority use cases) via toolbar customization:

* Proposal A (default to and enable existing combined button; offer new, separate buttons via toolbar customization for those who really need them)
(If we think that the *majority* of our users will primarily stick with one language and only rarely change to another language)
- Keep the existing combined button as a default
- Enable the existing combined button for subject (original intention of this bug)
  - which enables language selection in subject (!)
- Enable spell-check dialog for subject in a followup bug
- Add Jorg's 2 buttons to the customization palette
  1 spell-only button
  2 language-selector-only button (yeah, showing the current language *for those who care* might be a win)

* Proposal B (default to new, separate buttons; offer enabled existing combined button via toolbar customization)
(If we think that the majority of our users regularly change their language AND always want to see which language they are in)
- Make Jorg's 2 buttons the default
  1 spell-only button
  2 language-selector-only button (showing the current language *for everyone* even those who don't care)
- Move the existing combined button into customization palette
- Enable the existing combined button for subject (original intention of this bug)
  - which enables language selection in subject (!)
- Enable spell-check dialog for subject in a followup bug

* Proposal C (default to new, separate buttons and dump the existing, combined button - current patch)
(If we think that the majority of our users regularly change their language AND always want to see which language they are in, AND we don't care about those other users (minority??) who change their language only once in a while)
- Make Jorg's 2 buttons the default
  1 spell-only button
  2 language-selector-only button (showing the current language *for everyone* even those who don't care)
- Dump the existing combined button (forcing all types of users to have the full-fledged language selector button if they want language selection from toolbar)
- Enable spell-check dialog for subject in a followup bug

My ranking (ThomasD):
Proposal A - best
Proposal B - acceptable
Proposal C - bad (no need to dump the current practical combined button if we can just keep it in customization palette)

Comment 172

[Thomas D. (:thomas8)]

Fwiw and FTR, the scope of this bug was significantly morphed (reduced) by Jorg K just before comment 58:

Original summary: Spell check command disabled in subject line (button, main menu "Check Spelling", keyboard shortcut Ctrl+Shift+P)

Morphed summary: Selection of spell check dictionaries disabled in subject line

Comment 0 isn't very clear, but starting from reporter's comment 2 with its summary this bug just wanted to enable the *combined* spell button for subject to allow dialogue-spell-checking AND language selection in subject. So this bug has been somewhat hijacked for the language selector part only, and then expanded on that into the realm of other existing bugs. Iow, we've lost track of enabling the spell-check-dialogue for subject which was a major part of this bug. Anyway, I guess that's now for a follow-up bug.

Comment 173

[Jorg K (CEST = GMT+2)]

It is correct that the focus of this bug from 2007 (8 1/2 years old!!) has shifted after an initial investigation showed that:
a) Doing a full spell check on the subject isn't a trivial task, and it is questionable whether it's
   desirable at all.
b) Leaving the existing (combined) "Spelling" button always enabled isn't a good option either.

Therefore the conclusion was to have a context-sensitive button for "Spelling" and an always available button for "Language" choice, which can also be used as language indicator.

Frankly, I haven't seen the reporter on this bug for 8 years, so no one has complained about the hijacking. However, we have other active participants who see their ship come in (comment #144) who have joined the discussion.

Thomas, can you please explain to me why the people who do not want to have the language button can not just remove it from the user interface. I would estimate that some 30% of Thunderbird users write in their mother tongue and additionally in English. The "Christmas" users can remove the button and change the language via the right-click (context) menu in subject or body.

Attempting to make a good start and responding to your proposals:

A: Old button stays (enabled in subject), new buttons available for customisation.
This is not acceptable, since users will not find the buttons.
I fought the "Correspondents" column a little and was told that innovation makes no sense if you can't see it. Therefore we now have a preference to disable the innovation (bug 1152706).

B: New buttons come in, old button (enabled in subject) available for customisation.
That's OK, however, the old button would stay as is. Enabling the old button in the subject is non-trivial, since no one can answer the question what should be done if it is clicked.

C: As per the patch. People who don't need the language button can remove it.

Your cases A to C also have "Enable spell-check dialogue for subject in a follow up bug". IMHO I won't live long enough to see that implemented.

So if we all agree, I can provide a patch for B(mod):
New buttons come in, old button (with the exact same defective functionality) available for customisation for the people who want to maintain the existing status quo.

IMHO 100% of all users who care would gain from this solution, including the reporter, how appears to be Czech and does write in two languages, 0% of users would experience any worsening.

Comment 174

[Richard Marti (:Paenglab)]

(In reply to Jorg K from comment #167)
> (In reply to Richard Marti (:Paenglab) from comment #165)
> 
> > Please can you move the dictionary text to the end of the status bar?
> 
> Hmm, to the end, you mean the right. Not really without drilling it open a
> whole lot more.
> At the end, we show the encryption status, so a little envelop with a red
> seal.

I'm a little bit influenced by https://addons.mozilla.org/thunderbird/addon/dictionary-switcher/ which placed the switcher at this point :). Unfortunately the language guess is no more working in TB.

Comment 175

[Jorg K (CEST = GMT+2)]

I've been looking into proposal B:

It's hard to maintain two spelling buttons, since the dictionary popup list is identified via an ID:
<menupopup id="languageMenuList" oncommand="ChangeLanguage(event);"
  onpopupshowing="OnShowDictionaryMenu(event.target);"/>
To have two spelling buttons I'd have to introduce two lists and modify the code here
https://dxr.mozilla.org/comm-central/source/mail/components/compose/content/MsgComposeCommands.js#3065
to update both lists. From a maintanance point of view, that seems like a nightmare. One would have to explain why the old button is left behind.
I'd also have to touch code that is shared by SeaMonkey here:
https://dxr.mozilla.org/comm-central/source/editor/ui/composer/content/editor.js#2283
Just impossible.

Conclusion: I'm unassigning myself from the bug. It's impossible to implement a solution that is technically feasible and can reach consensus.

Whoever feels inclined, can pick up from here. To land the static language button, one would just have to remove the code in /mail/components/compose/content/MsgComposeCommands.js from the patch.

Comment 176

[Jorg K (CEST = GMT+2)]

Comment on attachment 8647727 [details] [diff] [review]
Proposed patch to have two independent buttons, dynamic label (v15, refreshed again, CSS un-bitrotted)

Carrying forward ui-review+ from Richard and Blake.
Carrying forward review+ from Magnus, however, he explicitly asked for the removal of the dynamic label, which is technically covered by the review+, but not from Magnus' view of the user interface.

Comment 177

[Magnus Melin [:mkmelin]]

(In reply to Jorg K (No more Thunderbird contributions) from comment #158)
> So how do we settle this?

I don't actually see any of the ui people objecting to the static label (like I proposed). 
If they do, I'd like justifications for having a toolbar button that would behave differently from any other toolbar button in the application.

Comment 178

[Jorg K (CEST = GMT+2)]

(In reply to Richard Marti (:Paenglab) from comment #174)
> I'm a little bit influenced by
> https://addons.mozilla.org/thunderbird/addon/dictionary-switcher/ which
> placed the switcher at this point :). Unfortunately the language guess is no
> more working in TB.

I didn't know this add-on. I looked at it in the DOM inspector in TB31. It inserts some stuff between the signing/encryption status and the resizer (that's what I meant with "drilling down"). I contacted the author and told them what has changed in TB38 and what they need to do to fix their add-on. It achieves what my patch was meant to do. Perhaps Magnus is right and the whole thing is better covered in an add-on.

(In reply to Magnus Melin from comment #177)
> (In reply to Jorg K (No more Thunderbird contributions) from comment #158)
> I don't actually see any of the ui people objecting to the static label
> (like I proposed). 
> If they do, I'd like justifications for having a toolbar button that would
> behave differently from any other toolbar button in the application.

That is correct. In fact, Blake gave ui-r+ on the static label (comment #109). However in comment #113 he said: "I like the dynamic label, if you can get it working. But if you can't, using "Language" would be an acceptable fallback."

Richard gave the dynamic label ui-review+ in comment #138 and said: "I think showing the selected language is a win. The additional language info in the brackets isn't needed here."

IMHO that rather wide "Language" button is not worth having, if the screen real estate is just being wasted to make a language choice possible in the subject. That can be achieved with a right click (context menu) (which is a little more tedious than hitting a big button). The added nicety of the solution lies in actually showing the language in use at a glance, since the setting determines the behaviour of the inline spell check. A button is also closer to where you want to start typing (subject, body) than a status bar.

You are correct that this button would behave differently to other buttons in the application, but there is already a range of behaviours:
The "Send" button changes its label in offline mode to "Send Later".
The "rich text" editing buttons for colour, bold, italics and underline do show the selected state, as do the (un)numbered lists.
The "Security" button sets indicators (red seal/lock icons) in the status bar.

Frankly, I'm a little tired of this bug. I started investigating an 8 y/o bug at comment #39, now we're at comment #177 and counting.

May the powers that be decide what they want to do. Comment #175 describes what needs to be done for a static label. Surely add-ons could hook into that easily, but I don't see the point of yet another add-on. I believe that users will fall into two classes: Single language users will remove the button, multi-language users (excluding Thomas) will like it and appreciate the extra functionality out of the box.

If you think that confused single-language users are in the majority, you could also just provide the language button in the customisation area, so people who want it, can pull it out, all the others won't even notice the the spelling button lost the drop down arrow, since they never used it.

Needless to say that is could safely be tried out in Daily/Aurora/Beta before it came out in TB45 in March 2016.

Comment 179

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (No more Thunderbird contributions) from comment #178)
> You are correct that this button would behave differently to other buttons
> in the application, but there is already a range of behaviours:
> The "Send" button changes its label in offline mode to "Send Later".
The "Attach" button does not change a "state" so no dynamic label in necessary.
The "Security" button does not change the label since it controls two states:
Signed and Encrypted, so there would be four awkward combined states to display.
Plus it wouldn't be clear that under you'd find "View Security Info" under "Signed and Encrypted", if you changed the label. However, the states are displayed in the status bar.
The "Save" button controls a state "Draft"/"Template" and has an action. Once again, it does not lend itself to having a dynamic label.

The only button that would lend itself to a dynamic label is the "Language" button, especially if placed next to the "Spelling" button, so that they form a (loose) "pair", as was pointed out in comment #159. (BTW, thanks for the comment, but forming a tighter pair/group is not possible with the UI elements supported by Mozilla).

Comment 180

[Richard Marti (:Paenglab)]

Magnus, it crossed now my mind, we have already dynamic buttons: the Junk/No Junk- and the Smart Reply buttons. So I think this reason can be canceled.

Comment 181

[Thomas D. (:thomas8)]

(In reply to Jorg K - let's discuss some more so it never gets done ;-) from comment #173)
> It is correct that the focus of this bug from 2007 (8 1/2 years old!!) has
> shifted after an initial investigation showed that:
> a) Doing a full spell check on the subject isn't a trivial task, and it is
> questionable whether it's
>    desirable at all.

I wonder how hard it really is; definitely desirable. A typical case of ux-implementation-level:

> Interfaces should not be organized around the underlying implementation and technology in ways that
> are illogical, or require the user to have access to additional information that is not found in the
> interface itself.

Users don't care that technically, subject and body are different spellcheck ranges. We have several bugs suggesting that they are (rightly) expecting subject and body to be considered a single spell check range (e.g. this bug 368915, bug 477073, bug 433181, bug 717292, etc.); Jorg himself has actually contributed patches to that end (e.g. bug 967494, which fixed bug 717292). I can't believe we're not able to get the spell-check dialog to work for subject with reasonable effort, and to find some way of linking that up with the body check if we think it should be linked. Technicalities shouldn't hamper good UX design; work around them or fix them. If we can't do it ourselves, get the right people like Ehsan on board. Imo, spell-check dialogue triggered from subject should consider the subject selected and after checking that, there should be a question "Finished spell-checking subject. Continue with message body?". The reverse case, starting out from selection-less body spell check e.g. with "spell check before sending", should just consider subject and body a single spell check range (ux-error-prevention). I think that's not too much to expect in 2015. (And FTR, spell-check *dialogue* is still important functionality in spite of inline spell checking, for all those users who want to quickly write along first without distraction and focus on spelling details later in a coherent, systematic, and time-efficient approach; some functions like "ignore all" are not even available from inline spelling, which may or may not be a bug).

And FTR, it was only Jorg who at a very early stage declared and insisted that he would not pursue solutions that just enable the combined spell button for subject (comment 68), in spite of Neil offering technical help (e.g. comment 67), and Blake even suggesting to make the enabled spell button part do nothing for now as the "least-bad alternative" from "terrible alternatives" (comment 87).

> b) Leaving the existing (combined) "Spelling" button always enabled isn't a
> good option either.

Well, somewhat in line with Blake's comment 87, I disagree (regardless if we keep the combined button as default or just via customization, if we make Jorg's 2 separate buttons the default). Enabling the existing combined spelling button for subject solves the main problem of this bug, to enable users to choose the language before starting to write any text. Starting the dialogue spell-check from subject is possibly a less frequent scenario (but we need to look into this anyway as explained above). Doing nothing when spell-check (dialogue) is triggered from subject is not much worse than now where that function is unexpectedly disabled. If we don't like doing nothing, an honest error message, and if we are ashamed of that, let's try to fix the whole mess (see bugs referenced above).

> Therefore the conclusion was to have a context-sensitive button for
> "Spelling" and an always available button for "Language" choice, which can
> also be used as language indicator.
> 
> Frankly, I haven't seen the reporter on this bug for 8 years, so no one has
> complained about the hijacking. However, we have other active participants
> who see their ship come in (comment #144) who have joined the discussion.

The people really waiting for this ship are actually very few judging from BMO data; comment 144 is by the reporter of bug 491055 filed 2009 with one duplicate (408207); on both bugs, hardly any activity and no affirming user comments at all. What really bothers lots of users is that they can't work with multiple languages and/or multiple (specialized topical or cross-language) dictionaries for the same document (Bug 69687, 61 votes, 31 duplicates, though it's a cesspool which is not clearly defined; Bug 676500, Bug 481884; all of these Core bugs but the scenarios are all but exclusively for Thunderbird).
So users actually want to tick *more than one* dictionary on the list, say to check both English and Spanish at the same time (fuzzy intersections possible, but limited); or to check a specialized medical English dictionary in addition to the standard English dictionary. The dynamic single-language button label proposed in this bug 368915 actually works against that highly requested feature.

For the avoidance of doubt:
* I'm OK with introducing the two separate buttons for Spell and Language selection (if we keep the existing combined button *at least* via customization)
* I think Magnus point of ux-consistency is not really applicable as others have explained above
* But I can't quite see the benefit of making the two new buttons the default, which imo violates ux-minimalism for the majority of our users who are rarely if ever changing their language;
* And I have doubts if the dynamic label is really as helpful as it's portrayed to be (see above); in fact it seems to make future improvement harder wrt much-wanted Bug 69687, Bug 676500, Bug 481884.

> Thomas, can you please explain to me why the people who do not want to have
> the language button can not just remove it from the user interface. I would
> estimate that some 30% of Thunderbird users write in their mother tongue and
> additionally in English. The "Christmas" users can remove the button and
> change the language via the right-click (context) menu in subject or body.

* Changing language from subject context menu is currently NOT implemented (new bug recommended).
* If we agree (as you seem to), that the majority of our users are *NOT regularly* (or even never) changing the language, it means that for them the big new language selector button is just permanent clutter on the composition toolbar (ux-minimalism).
* Which doesn't mean they *never* want to change or check the language; having the small-target dropdown language selector nicely tucked away in-place is just right, ux-minimalistic, and intuitive (spell settings on a spell button). Why force them into accepting permanent visual clutter or using less discoverable patterns like body context menu when we can easily continue to provide the combined button (at least) via toolbar customization palette, as I shall show below?
* The main issue is with *selecting* the correct language, which will typically happen maximally ONCE per composition. Thereafter, showing the current, correct spell-check language has very little informational value (ux-minimalism). I KNOW that I'm writing in German, and if only 3% of my text are marked red for wrong spelling, I'll just fix those errors without double-checking the language attribute. Whereas as long as you have the /wrong/ language selected, near 100% of your text will be red, so again you'll know it's the wrong language without looking at the current language attribute. How does it help then to see the *wrong* language shown on the button?
* It seems actually slightly easier (more memorable) to always go for neutral "Language" label regardless of which wrong(!) language is currently set. The number of clicks required to change language are the same either way (dynamic or static label).

> Attempting to make a good start and responding to your proposals:
> 
> A: Old button stays (enabled in subject), new buttons available for
> customisation.
> This is not acceptable, since users will not find the buttons.

Permanent visual clutter without much purpose for the majority is also unacceptable; functionality is fully available and discoverable from the current combined button; I'm not sure what's "unacceptable" about providing advanced functionality via toolbar customization. I think we could consider users who need to change their language so regularly that they always want to see it "advanced users", who'll probably search for that feature if they need it, or not?

> I fought the "Correspondents" column a little and was told that innovation
> makes no sense if you can't see it. Therefore we now have a preference to
> disable the innovation (bug 1152706).
> 
> B: New buttons come in, old button (enabled in subject) available for
> customisation.
> That's OK, however, the old button would stay as is. Enabling the old button
> in the subject is non-trivial, since no one can answer the question what
> should be done if it is clicked.

As I have laid out above, we need to fix the spell-check-scope issues between subject and body anyway; lacking which, the UX-lead could live with a temporary solution which fully enables the combined button and does nothing when starting dialogue-spell-check in subject (probably less frequent scenario).

> C: As per the patch. People who don't need the language button can remove it.
> 
> Your cases A to C also have "Enable spell-check dialogue for subject in a
> follow up bug". IMHO I won't live long enough to see that implemented.

Miracles do happen and sometimes those age-old bugs deemed difficult even turn out to be one-liners. We shouldn't give up before we even started. I'm hoping that Jorg will live long because he is a great contributor :)

> So if we all agree, I can provide a patch for B(mod):
> New buttons come in, old button (with the exact same defective
> functionality) available for customisation for the people who want to
> maintain the existing status quo.
> 
> IMHO 100% of all users who care would gain from this solution, including the
> reporter, how appears to be Czech and does write in two languages, 0% of
> users would experience any worsening.

So with Thomas for A and Jorg for B, let's hear from others what they'd prefer in view of the UX analysis provided in this comment. Based on this analyis, I think C (forcing the 2 new buttons on everyone while dumping the existing combined button for good, even from customization) can safely be discarded because Jorg's subsequent arguments against B (where he changed his mind again) are also technically incorrect:

(In reply to Jorg K - let's discuss some more so it never gets done ;-) from comment #175)
> I've been looking into proposal B:
> 
> It's hard to maintain two spelling buttons, since the dictionary popup list
> is identified via an ID:
> <menupopup id="languageMenuList" oncommand="ChangeLanguage(event);"
>   onpopupshowing="OnShowDictionaryMenu(event.target);"/>
> To have two spelling buttons I'd have to introduce two lists and modify the
> code here
> https://dxr.mozilla.org/comm-central/source/mail/components/compose/content/
> MsgComposeCommands.js#3065
> to update both lists. From a maintanance point of view, that seems like a
> nightmare. One would have to explain why the old button is left behind.

That's entirely not true. Technical correction:
It's dead-simple and practically zero maintenance burden to make/keep an alternative spelling button available if we go for A or B; in fact it'll make very clean code and we might even benefit from that for Bug 728775. In XUL, just move the popup out of the button and then attach the popup whereever it's needed using the 'popup' attribute of the parent element. No JS changes required; we'll just populate a single popup as before. I'll attach a demo XUL file in a following comment.

>   <toolbarbutton class="toolbarbutton-1" type="menu"
>                 id="spellingButton" label="Language"
>                 tooltiptext="spellingButton.tooltip"
>                 command="cmd_spelling"
>                 popup="languageMenuList">

> I'd also have to touch code that is shared by SeaMonkey here:
> https://dxr.mozilla.org/comm-central/source/editor/ui/composer/content/
> editor.js#2283
> Just impossible.

Huh? Are you saying that hiding one more button for SeaMonkey is an impossible task?

2283    HideItem("spellingButton");
2284    HideItem("menu_checkspelling");
2285    RemoveItem("sep_checkspelling");

> Conclusion: I'm unassigning myself from the bug. It's impossible to
> implement a solution that is technically feasible and can reach consensus.

You're entitled to that opinion and decision, but I don't see any drama here which can't be solved in a cooperative manner. Guess after 8 years with this little bug (7 votes, only one duplicate), no need to rush to conclusions but let's try to get it right instead.

> Whoever feels inclined, can pick up from here. To land the static language
> button, one would just have to remove the code in
> /mail/components/compose/content/MsgComposeCommands.js from the patch.

Thanks for that cooperative pointer, that's appreciated.
Feedback from others welcome.

Comment 182

[Thomas D. (:thomas8)]

(In reply to Thomas D. from comment #181)
> It's dead-simple and practically zero maintenance burden to make/keep an
> alternative spelling button available [via toolbar customization palette]
> if we go for A or B;

Fwiw, providing alternative buttons via toolbar customization palette also seems to be one of Blake's first proposals:

(In reply to Blake Winton (:bwinton) [Away until Aug 23rd at least.] from comment #76)
> I think it would be better to have "Spelling" and "Spelling+Language" as two
> different buttons, where the second one has the spell-check button plus the
> combo-dropdown.

Comment 183

[Thomas D. (:thomas8)]

Attachment: Demo1.xul - Proof of concept: Reusing same popup for several buttons

Comment 184

[Thomas D. (:thomas8)]

(In reply to Thomas D. from comment #181)

> (In reply to Jorg K - let's discuss some more so it never gets done ;-) from
> comment #175)
> > I've been looking into proposal B:
> > 
> > It's hard to maintain two spelling buttons, since the dictionary popup list
> > is identified via an ID:
> > <menupopup id="languageMenuList" oncommand="ChangeLanguage(event);"
> >   onpopupshowing="OnShowDictionaryMenu(event.target);"/>
> > To have two spelling buttons I'd have to introduce two lists and modify the
> > code here
> > https://dxr.mozilla.org/comm-central/source/mail/components/compose/content/
> > MsgComposeCommands.js#3065
> > to update both lists. From a maintanance point of view, that seems like a
> > nightmare. One would have to explain why the old button is left behind.
> 
> That's entirely not true. Technical correction:

Outch. I spoke too soon; the technical correction isn't working correctly as yet.
Just adding the popup attribute to the combined toolbarbutton will open the popup even when clicking on the main button part; however, I'm sure that can be fixed if there's a will. It's actually a design shortcoming of the xul for the button where keeping functionality/properties of main and dropdown part apart is way too hard (existing bugs).

But still, even if we'd need to attach the language menu via JS to two different buttons with two different popup menu IDs, that's still very doable with a couple of lines.

> It's dead-simple and practically zero maintenance burden to make/keep an
> alternative spelling button available if we go for A or B; in fact it'll
> make very clean code and we might even benefit from that for Bug 728775. In
> XUL, just move the popup out of the button and then attach the popup
> whereever it's needed using the 'popup' attribute of the parent element. No
> JS changes required; we'll just populate a single popup as before. I'll
> attach a demo XUL file in a following comment.
> 
> >   <toolbarbutton class="toolbarbutton-1" type="menu"
> >                 id="spellingButton" label="Language"
> >                 tooltiptext="spellingButton.tooltip"
> >                 command="cmd_spelling"
> >                 popup="languageMenuList">

Comment 185

[Thomas D. (:thomas8)]

(In reply to Jorg K - let's discuss some more so it never gets done ;-) from comment #179)
> (In reply to Jorg K (No more Thunderbird contributions) from comment #178)
> > You are correct that this button would behave differently to other buttons
> > in the application, but there is already a range of behaviours:

+1 to most of this comment

> The "Save" button controls a state "Draft"/"Template" and has an action.
> Once again, it does not lend itself to having a dynamic label.

Not quite. The "Save" button is an action button controlling/remembering one out of *three* different states: Save as file, Save as Draft, Save as Template.
Having a dynamic label indicating the current state would actually make sense here as it's not very clear from subsequent clicks in which flavor it'll save. I once saw to it that at least the current state is visible on the dropdown...

I think the dynamic label was just not realized for reasons of ux-minimalism, because it would be unnecessarily long and distracting, potentially changing button size, while most users will rarely use the file or template flavors and those who do probably know what they are doing. Also, preserving the last choice for subsequent saves makes a lot of sense, if you're saving as template and then edit more and save again, you'll probably want the changes in that same template and not suddenly in a file or draft.

Comment 186

[Jorg K (CEST = GMT+2)]

I don't have the time or inclination to answer all of this. So just a few points.

It's nice that you're a purist, but with this attitude, the bug will rest another eight years. Yes, the spelling button could be make to work as you describe, in subject and body, moving from one to the other. Nice. However, this bug is so old and so hard and none of the people who have the skill set considers it important enough (see for example comment #24). So we're looking for a pragmatic solution *now*. 1) People don't want to click in the body first to change the spell check language in the subject and then to return to the subject. Even if you fixed the spelling button, 2) there would be people who want to see immediately which language their text is spell checked in. Add-ons like "Dictionary switcher" prove that such people exist. The solution here fixes both. My last proposal was to not put the new button into the default set, so the only visible change would be the removed dropmarker.

Ehsan is a Mozilla employee, who works on M-C/Gecko. If he finds time, he fixes M-C editor bugs in M-C/editor/libeditor (https://dxr.mozilla.org/mozilla-central/source/editor/libeditor). For the this to happen, we need to provide test cases for Firefox. He has absolutely nothing, I repeat, *nothing*, to do with Thunderbird. I know, because I've been working with him since March 2015 to fix some long-standing editor bugs (bug 1100966, bug 1140617, bug 1169996, bug 967494, bug 756984, bug 1140105, bug 772796, and there are more to come).

You correctly realised that due to
https://dxr.mozilla.org/comm-central/source/mail/components/compose/content/MsgComposeCommands.js#3065
var languageMenuList = document.getElementById('languageMenuList');
you need two separate pop-up lists with two IDs.

Everything is doable, but it's just a maintenance nightmare, and I can just hear Magnus saying, "too complicated". He even thinks than an additional preference, one line to create and one line to query, is too much effort. (I don't see the necessity of such a preference, I just offered it as a compromise.)

Last not least:
> Changing language from subject context menu is currently NOT implemented.
Huh? Do I need to attach a screen shot to show that this *IS* working and since bug 967494 it is even synchronised with the body and the selection in the drop down menu?

Comment 187

[Jorg K (CEST = GMT+2)]

Summary:
- ui-review+ in comment #109 from Blake for static button,
  with the incentive to do a dynamic one (comment #113).
- ui-review+ in comment #138 from Richard for dynamic button.
- review+ from Magnus for technical solution in comment #146, however, not for the dynamic button
  as such, but with the code to support the dynamic button approved (apart from some nits).

I think Magnus' argument from comment #156 and comment #177 that a dynamic button would be "inconsistent" with the rest of the interface has been sufficiently disproven (see for example comment #180).

What else is needed to bring the matter to a close? Possibilities are:
1) Additional ui-reviews could be asked of Richard to ui-review- the static button,
   although it's pretty clear that he prefers the dynamic button
   (comment #113, comment #165, comment #180).
2) Additional ui-review from Blake for the dynamic button since he's never reviewed it.

If there are reviews missing, let's request them!

Comment 188

[Jorg K (CEST = GMT+2)]

Comment on attachment 8648489 [details]
Demo1.xul - Proof of concept: Reusing same popup for several buttons

Actually, proving the opposite.

Comment 189

[Jorg K (CEST = GMT+2)]

Summary continued:
Thomas suggested to maintain the "old" combined "Spelling" button as a configuration option.
This would entail more complex code and agreement from the SeaMonkey team.

I considered not adding the "Language" button to the default set, so only the existing "Spelling" button would lose the dropmarker. Users wishing to change language would have to enable the new button or use the context (right-click) menus.

Comment 190

[Thomas D. (:thomas8)]

(In reply to Jorg K - let's discuss some more so it never gets done ;-) from comment #186)
> I don't have the time or inclination to answer all of this. So just a few
> points.
> 
> It's nice that you're a purist, but with this attitude, the bug will rest
> another eight years.

I think this bug is at an advanced stage, so I don't believe just putting either the two new buttons or the old button into the customization palette and duplicating a single menu for that purpose is difficult enough to delay this for another 8 years, more so given your strong impetus to get this rolled out.

> Yes, the spelling button could be make to work as you
> describe, in subject and body, moving from one to the other. Nice. However,
> this bug is so old and so hard and none of the people who have the skill set
> considers it important enough (see for example comment #24). So we're
> looking for a pragmatic solution *now*.

We're on the same line here, I didn't ask for perfection in this bug, only to preserve the existing combined button while we add your new buttons.

> 1) People don't want to click in the
> body first to change the spell check language in the subject and then to
> return to the subject. Even if you fixed the spelling button, 2) there would
> be people who want to see immediately which language their text is spell
> checked in. Add-ons like "Dictionary switcher" prove that such people exist.
> The solution here fixes both. My last proposal was to not put the new button
> into the default set, so the only visible change would be the removed
> dropmarker.

Huh? That would mean you're removing the language selector entirely from the toolbar, which doesn't make sense. Why should we?

> Ehsan is a Mozilla employee, who works on M-C/Gecko. If he finds time, he
> fixes M-C editor bugs in M-C/editor/libeditor
> (https://dxr.mozilla.org/mozilla-central/source/editor/libeditor). For the
> this to happen, we need to provide test cases for Firefox. He has absolutely
> nothing, I repeat, *nothing*, to do with Thunderbird. I know, because I've
> been working with him since March 2015 to fix some long-standing editor bugs
> (bug 1100966, bug 1140617, bug 1169996, bug 967494, bug 756984, bug 1140105,
> bug 772796, and there are more to come).

Repeating doesn't change the fact that Ehsan's work is needed for Thunderbird to make progress in those areas... I'm not familiar with the structures of responsibility outside Thunderbird, but I think you very well got my point that we can ask others to help out when we need insider knowledge.

> You correctly realised that due to
> https://dxr.mozilla.org/comm-central/source/mail/components/compose/content/
> MsgComposeCommands.js#3065
> var languageMenuList = document.getElementById('languageMenuList');
> you need two separate pop-up lists with two IDs.

I said that's one way of doing it (and quite simple, too, as we're creating the popup lists dynamically and it's only a few lines where those menuitems get added to the popup which need duplication). The other way being to use the click targets to prevent the attached-via-attribute popup where it's not needed (also probably not too hard for someone who knows the stuff).

> Everything is doable, but it's just a maintenance nightmare,

Sorry, but I refuse to accept that sharing/duplicating the same menu between two buttons could possibly be a maintenance nightmare. You're exaggerating.

> and I can just
> hear Magnus saying, "too complicated". He even thinks than an additional
> preference, one line to create and one line to query, is too much effort. (I
> don't see the necessity of such a preference, I just offered it as a
> compromise.)

You didn't sound very compromising where you offered that, and imo a pref comes with a higher maintenance burden than a simple alternative button somewhere in XUL. I'm undecided on the dynamic label but the general tendency in the bug here and perhaps from addons seems in favor and I can live with that.

> Last not least:
> > Changing language from subject context menu is currently NOT implemented.
> Huh? Do I need to attach a screen shot to show that this *IS* working and
> since bug 967494 it is even synchronised with the body and the selection in
> the drop down menu?

You're right... I only had one dictionary installed so the language selector was indeed not present on context menu, which isn't helpful for ux-efficiency, bug 408209. Currently language selector becomes available only after installing an additional language, whereas it would be better to always have the language selector available with an "Download more dictionaries..." link at the end.

Btw (I think I also mentioned that before), not enabling the spelling command when in subject also disables some other functions which are only available from the spelling dialogue, like adding new words to the custom dictionary. There's no reason why users shouldn't be able to add/manage custom words when they start off in subject.

(In reply to Jorg K - let's discuss some more so it never gets done ;-) from comment #188)
> Comment on attachment 8648489 [details]
> Demo1.xul - Proof of concept: Reusing same popup for several buttons
> 
> Actually, proving the opposite.

No. Just proving that the split-button design in XUL is incomplete, so it takes some extra lines of codes to prevent popup depending on click targets or some such to work around that. It's possible.

(In reply to Jorg K - let's discuss some more so it never gets done ;-) from comment #189)
> Summary continued:
> Thomas suggested to maintain the "old" combined "Spelling" button as a
> configuration option.

Configuration option makes it sound unnecessarily complex. It's just a plain alternative button on customization palette for those who need it, which I believe is the majority which is why I suggested proposal A to make both of the separate new buttons go into customization instead.

> This would entail more complex code and agreement from the SeaMonkey team.

Omg, yes we'd add a line or two in SeaMonkey...

> I considered not adding the "Language" button to the default set, so only
> the existing "Spelling" button would lose the dropmarker. Users wishing to
> change language would have to enable the new button or use the context
> (right-click) menus.

That doesn't make sense for anyone, as explained at the beginning of this comment.

Comment 191

[Thomas D. (:thomas8)]

Anyway, I'm also getting tired of this discussion and I'm irritated by strong thrust with which Jorg is trying to push this through, which in turn makes me become defensive about possible alternatives in the interest of better long-term UX. But I can appreciate the personal frustration of somebody who has been affected by the current disfunctional design for the last 8 years or more, and who just wants this to get out of the way now without stopping for details.

Comment 192

[Magnus Melin [:mkmelin]]

It's not the dynamically per se that bothers me. It's that when you want to change the language you'd be looking for a Language button. If you're changing the language to Spanish, you're not looking for a button named English. As you can see there's a difference from say the Junk/Not Junk button or the Reply buttons.

BTW, there are many versions of English so, switching between them would need something more - or maybe that was covered, too tired to look it up...

So, while a bit geeky, I guess having the button dynamic but with the lang+country code showing is something that would work for me. I mean something like "Language: en-US".

Comment 193

[Jorg K (CEST = GMT+2)]

Hmm, yes, I like the geeky idea, but it's just too geeky.

Please consider this. I have four languages installed (sadly the French stuff comes as a bundle):
In the context menu (right click) I see this 
English (Australia)
English (United States)
French (classic / reform)
French (classic)
French (modern)
French (reform)
German (Germany)
Spanish (Spain)

In the drop-down I see:
English (AU)
English (US)
French (classic-reform)
French (classic)
French (modern)
French (reform)
German (DE)
Spanish (ES)

And finally in the full spell checker:
English/Australia)
English/United States
fr-classic-reform
fr-classic
fr-modern
fr-reform
German/Germany
Spanish/Spain

There is bug 728775 to unify this and I intend to look at it, if we ever fix this bug here.

Now you want to add another variant on the button:
en-AU
en-US
fr-classic-reform
fr-classic
fr-modern
fr-reform
de-DE
es-ES

I don't think that's going to be approved by any UX person.

The idea was to use only the language and not the specific variant to keep the button width mostly the same (although Thomas will tell us, that "Deutsch" and "Französisch" have quite different length). If you want the full information, you need to put it into the status bar. That would waste the space the the new button occupies.

> If you're changing the language to Spanish, you're not looking for a button named English.
Agreed. There is a 10 second discovery effect involved. There is a dropmarker to indicate that there is more. If the "English" is what is bothering you, you'd most likely click on the element and see what happens.

Anyway, the status bar solution also has ui-review+ from Richard (comment #165), although it's not his preferred solution. I'd have to move the info more to the right as he requested, just like the "Dictionary switcher" addon, which we would be practically absorbing.

So the two solutions:

Dynamic button:
Pros:
- Compact information, label changes when clicked, language info is available close to
  where the typing starts (subject, top of message), no eye-movement (saccade?) required.
Cons:
- Width of button/menu changes.
- Full information on language variant not visible.
- Not 100% intuitive at first use.

Static button with status display:
Pros:
- Fixed button/menu size.
- Full information on language variant visible.
- UX more aligned with "Security" button, which also puts extra stuff into the status bar.
- More aligned with MS/Open Office (although no right-click available).
Cons:
- Need to look at two places at the same time.
- Button space not used.
- Not visible if status bar hidden.

Hmm, perhaps the status bar solution is not so bad after all and I dismissed it too hastily.

If however we go for a status bar display, we really need to ask ourselves (and Thomas will cheer) whether we need an additional button. We could just always enable the "Spelling" button and the user will soon discover that is checks the message body and nothing else.

Great, coming full circle here ... ;-)

Comment 194

[Jorg K (CEST = GMT+2)]

Attachment: Coming full circle: Spelling button always on, language feedback in status bar, easy as pie ;-)

Having already explored all possible avenues, I had various code fragments lying around. So here's a version with the old "Spelling" button always enabled, and additionally a status bar feedback. This time, it has its own "panel", so the text does not get overwritten by anyone else.

The text display is permanent; I could add a variation for not displaying it, if only one dictionary is installed (which is almost never the case from experience, since English typically installs a few flavours, not to mention Spanish, which installs, at least on Linux, a whole swag of different flavours of Spanish for all the different Latin American countries). Well, maybe German is unique, at least for users in Germany.

Let me know that you think guys. The question is: Can we settle this bug with less than 200 comments ;-)

Comment 195

[Jorg K (CEST = GMT+2)]

Attachment: Screenshot, sadly, caret doesn't show, but it was in the subject

Comment 196

[Magnus Melin [:mkmelin]]

Comment on attachment 8648523 [details] [diff] [review]
Coming full circle: Spelling button always on, language feedback in status bar, easy as pie ;-)

Review of attachment 8648523 [details] [diff] [review]:
-----------------------------------------------------------------

I'm fine with this, but yes not showing when we only have one dict would be better - what you get on windows out of the box is just en-US.

Comment 197

[Richard Marti (:Paenglab)]

Comment on attachment 8648523 [details] [diff] [review]
Coming full circle: Spelling button always on, language feedback in status bar, easy as pie ;-)

I'm okay with this when hidden with only one language installed.

Comment 198

[:aceman]

(In reply to Magnus Melin from comment #192)
> It's not the dynamically per se that bothers me. It's that when you want to
> change the language you'd be looking for a Language button. If you're
> changing the language to Spanish, you're not looking for a button named
> English.

In MS Word, this is exactly how it works. You can click the current language in the status bar to switch to another one. But yes, they also have another Language button in the toolbar, that does the same.

Comment 199

[:aceman]

Comment on attachment 8648523 [details] [diff] [review]
Coming full circle: Spelling button always on, language feedback in status bar, easy as pie ;-)

Review of attachment 8648523 [details] [diff] [review]:
-----------------------------------------------------------------

This version seems simple and clean. Thanks.
I like the status bar display of the language. Maybe the element could have a tooltip saying what the language actually means (i.a. that it is the language used for spell checking).

::: mail/components/compose/content/MsgComposeCommands.js
@@ +708,5 @@
>  
> +    cmd_spelling: {
> +        isEnabled: function() {
> +        dump ("nsSpellingCommand:isCommandEnabled\n");
> +        return true;

Why does this solve the problem, i.e. always enable the Spelling button? The button got enabled also without this code, but only in the editor. Is this overriding some code? May there be some clashes? Or is this code only called when outside the editor?

@@ +2128,5 @@
> +  // Observe the language attribute so we can update the language button label.
> +  gLanguageObserver = new MutationObserver(function(mutations) {
> +    mutations.forEach(function(mutation) {
> +      if (mutation.type == "attributes" && mutation.attributeName == "lang") {
> +        updateLanguageInStatusBar();

Do you have the value of the language available at this point so you could pass it into updateLanguageInStatusBar() ?

@@ +3205,5 @@
> +    if (item.getAttribute("value") == language) {
> +      document.getElementById("languageText").label = item.getAttribute("label");
> +      break;
> +    }
> +    item = item.nextSibling;

You could iterate a menulist using menulist XUL methods, e.g. getItemAtIndex(index) to abstract away how exactly are the DOM elements laid out in the menu.

::: mail/components/compose/content/messengercompose.xul
@@ +1091,5 @@
>                      class="statusbarpanel-progress"
>                      collapsed="true">
>        <progressmeter id="compose-progressmeter" class="progressmeter-statusbar" mode="normal" value="0"/>
>      </statusbarpanel>
> +    <statusbarpanel id="languageText" flex="1"/>

The languageText panel can probably be smaller than the main statusText, so make it flex less. It does not need to take 50% of the status bar, the same as statusText.

Comment 200

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #199)
> Maybe the element could have
> a tooltip saying what the language actually means (i.a. that it is the
> language used for spell checking).
The two security icons don't have a tooltip. A tooltip requires more translation effort.
I think a tooltip on "status" information is overkill.
However, I'll add one if the UI-review says so.

> Is this overriding some code?
Yes, this overrides the editor code. I can't move this, since it's used by SeaMonkey.

> > +  // Observe the language attribute so we can update the language button label.
> > +  gLanguageObserver = new MutationObserver(function(mutations) {
> > +    mutations.forEach(function(mutation) {
> > +      if (mutation.type == "attributes" && mutation.attributeName == "lang") {
> > +        updateLanguageInStatusBar();
> Do you have the value of the language available at this point so you could
> pass it into updateLanguageInStatusBar() ?
I don't get the question, or perhaps you miss the point.
updateLanguageInStatusBar() is not called here, it is defined to be called as part of the MutationObserver. The idea is, that whenever the "lang" attribute is changed, and these days it is changed in a lot of places, the information updates. The alternative is to visit all those places and make them call the update.

> You could iterate a menulist using menulist XUL methods, e.g.
> getItemAtIndex(index) to abstract away how exactly are the DOM elements laid
> out in the menu.
My stuff was inspired by InitLanguageMenu(), which uses DOM calls.
I don't know those XUL methods and I also can't find any documentation other than:
https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/menulist
I can't see a method/property to return the number of items.

> The languageText panel can probably be smaller than the main statusText, so
> make it flex less.
Coming up soon ;-)

Comment 201

[:aceman]

(In reply to Jorg K - let's discuss some more so it never gets done ;-) from comment #200)
> I think a tooltip on "status" information is overkill.
> However, I'll add one if the UI-review says so.
Sure, no problem.

> > > +  // Observe the language attribute so we can update the language button label.
> > > +  gLanguageObserver = new MutationObserver(function(mutations) {
> > > +    mutations.forEach(function(mutation) {
> > > +      if (mutation.type == "attributes" && mutation.attributeName == "lang") {
> > > +        updateLanguageInStatusBar();
> > Do you have the value of the language available at this point so you could
> > pass it into updateLanguageInStatusBar() ?
> I don't get the question, or perhaps you miss the point.
> updateLanguageInStatusBar() is not called here, it is defined to be called
> as part of the MutationObserver. The idea is, that whenever the "lang"
> attribute is changed, and these days it is changed in a lot of places, the
> information updates. The alternative is to visit all those places and make
> them call the update.
"At this point" I mean when the mutation happens and the observer is called and then you call 
updateLanguageInStatusBar. At that moment, is there something like mutation.attributeValue that would contain the language, e.g. "en-US". Currently you retrieve it via document.documentElement.getAttribute("lang") inside updateLanguageInStatusBar().

> > You could iterate a menulist using menulist XUL methods, e.g.
> > getItemAtIndex(index) to abstract away how exactly are the DOM elements laid
> > out in the menu.
> My stuff was inspired by InitLanguageMenu(), which uses DOM calls.
Yeah, I see. Often it works both ways. But using the XUL methods can abstract away some details of how the elements should be nested and can skip unrelated elements in the widget. I can remember an example like this:
<radiogroup>
<radio/>
<hbox>
<radio/>
</hbox>
</radiogroup>

This happens in pre-coded xul files (e.g. the account manager or preferences), where people can insert <*box> elements freely for formatting reasons. Iterating thoses radios in some distant code using .nextSibling would then break.

Of course, dynamically generated uniform menus should not have such unexpected elements. And using direct DOM methods is probably faster.

> I don't know those XUL methods and I also can't find any documentation other
> than:
> https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/menulist
> I can't see a method/property to return the number of items.
There is .itemCount.

Comment 202

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch: Spelling button always enabled, status info if more than one dictionary installed.

One note to the reviewer:
Not to add the observer, if there is only one dictionary at window create time is not an option, since the user can add more dictionaries while composing a message.

There is one quirk here:
If you disable or remove the active dictionary, call it "XX", while you write a message in that language, the status text will still say "XX" as it reflects the document's "lang" attribute, which hasn't changed. In fact, if you type into the window, you will see, that spell checking *continues* in the disabled/removed language. So the status information is quite correct.

All becomes good when selecting another language from the "Spelling" button, or, of course, starting a new composition window. BTW, I also fixed the "checking" on the drop-down menu of the spelling button, since after removal of the active dictionary, no language was selected and, at least in the en-US version which has an "inbuilt" en-US dictionary, this became "checked" although spell checking continued in "XX".

I think, disabling/removing the active dictionary is rather foolish, and the system reacts sufficiently well.

Comment 203

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #201)
> "At this point" I mean when the mutation happens and the observer is called
> and then you call 
> updateLanguageInStatusBar. At that moment, is there something like
> mutation.attributeValue that would contain the language, e.g. "en-US".
> Currently you retrieve it via document.documentElement.getAttribute("lang")
> inside updateLanguageInStatusBar().
Actually, there isn't, at least not according to this documentation (or I'm blind, which has also happened):
https://developer.mozilla.org/en/docs/Web/API/MutationObserver
The is only oldValue.

Anyway, I'm open to a better solution. Let's see what Magnus' review comes up with.

Comment 204

[:aceman]

Comment on attachment 8648658 [details] [diff] [review]
Proposed patch: Spelling button always enabled, status info if more than one dictionary installed.

Review of attachment 8648658 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/compose/content/MsgComposeCommands.js
@@ +3210,5 @@
> +  let noDisplay = (item == languageMenuList.lastChild);
> +  while (item) {
> +    if (item.getAttribute("value") == language) {
> +      document.getElementById("languageText").label =
> +        (noDisplay ? "" : item.getAttribute("label"));

This looks really complicated.

What about:
 if (languageMenuList.itemCount < 2)
   document.getElementById("languageText").label = "" (or just .collapsed=true on the element/panel)
 else while (item) ...

::: mail/components/compose/content/messengercompose.xul
@@ +1091,5 @@
>                      class="statusbarpanel-progress"
>                      collapsed="true">
>        <progressmeter id="compose-progressmeter" class="progressmeter-statusbar" mode="normal" value="0"/>
>      </statusbarpanel>
> +    <statusbarpanel id="languageText" flex="0.3"/>

Flex value can't be fractional. You have to increase the value at statusText.

Comment 205

[Richard Marti (:Paenglab)]

Comment on attachment 8648658 [details] [diff] [review]
Proposed patch: Spelling button always enabled, status info if more than one dictionary installed.

With only one dictionary installed where is still some space reserved for the panel in the statusbar. With aceman's proposal to collapse it it should be okay then.

And please add a tooltip which describes for what it is.

ui-r- for a new iteration.

(In reply to :aceman from comment #204)
> Comment on attachment 8648658 [details] [diff] [review]
> Proposed patch: Spelling button always enabled, status info if more than one
> dictionary installed.
> 
> Review of attachment 8648658 [details] [diff] [review]:
> -----------------------------------------------------------------
> ::: mail/components/compose/content/messengercompose.xul
> @@ +1091,5 @@
> >                      class="statusbarpanel-progress"
> >                      collapsed="true">
> >        <progressmeter id="compose-progressmeter" class="progressmeter-statusbar" mode="normal" value="0"/>
> >      </statusbarpanel>
> > +    <statusbarpanel id="languageText" flex="0.3"/>
> 
> Flex value can't be fractional. You have to increase the value at statusText.

You could also remove the flex, then the panel uses only the space for the text.

Comment 206

[Richard Marti (:Paenglab)]

Attachment: oneDictionary.png

How it looks with only one dictionary on Win10.

Comment 207

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch: Spelling button always enabled, status info if more than one dictionary installed (v3)

Addressing Aceman's and Richard's issues:
- removed complicated code.
- removed flex (implies, flex=0).
- collapsed, if only one dictionary.

Comment 208

[Jorg K (CEST = GMT+2)]

Oops, wires crossed here, it's all address apart from the tooltip. Coming up.

Comment 209

[Jorg K (CEST = GMT+2)]

Attachment: Proposed patch: Spelling button always enabled, status info if more than one dictionary installed (v4)

All addressed, I meant to say, now with tooltip.

Comment 210

[Jorg K (CEST = GMT+2)]

Note to the purists:
I placed the tooltip text into the section entitled "Mail Toolbar Tooltips", simply because there is no section for status bar tooltips.

Comment 211

[Richard Marti (:Paenglab)]

Comment on attachment 8648685 [details] [diff] [review]
Proposed patch: Spelling button always enabled, status info if more than one dictionary installed (v4)

Now it looks good.

Comment 212

[Jorg K (CEST = GMT+2)]

Thanks, Richard, finally.

Note to Magnus:
I should test for null after:
+  let languageText = document.getElementById("languageText");
in case the element gets misconfigured.

I'll add this once you have reviewed the patch.

Comment 213

[Thomas D. (:thomas8)]

Comment on attachment 8648525 [details]
Screenshot, sadly, caret doesn't show, but it was in the subject

(In reply to Jorg K - let's discuss some more so it never gets done ;-) from comment #195)
> Created attachment 8648525 [details]
> Screenshot, sadly, caret doesn't show, but it was in the subject

Yes, this is nice (enabling the existing combined button + status bar language display). Thanks Jorg for being creative and responsive in our cooperative pursuit of better UX.

Nit: Please visually de-emphasize the permanent status bar display of current spell check language by using dark-grey text (same color as Account name in From field), per ux-minimalism, ux-visual-hierarchy. Black text in status bar for something which is usually changed and looked at only once per composition is too distracting and screaming.

ui-r=me with that fixed.

(In reply to :aceman from comment #198)
> In MS Word, this is exactly how it works. You can click the current language
> in the status bar to switch to another one.

...and switching the language from status bar in MS Word works like a charme, very convenient because you don't have to search for it between all the other options. So we could just try the same... :)

Comment 214

[:aceman]

(In reply to Thomas D. from comment #213)
> (In reply to :aceman from comment #198)
> > In MS Word, this is exactly how it works. You can click the current language
> > in the status bar to switch to another one.
> 
> ...and switching the language from status bar in MS Word works like a
> charme, very convenient because you don't have to search for it between all
> the other options. So we could just try the same... :)

Yeah, but in MS Word all status bar widgets are clickable. In TB that is an exception (e.g. the offline icon). So I was commenting about the label of the thing, not that it must be clickable regardless of the position of the button. We need to keep some consistency :)

Comment 215

[Thomas D. (:thomas8)]

Attachment: Demo2.xul: statusbar language selector (like MS Word)

(In reply to Thomas D. from comment #213)
> Comment on attachment 8648525 [details]
> (In reply to :aceman from comment #198)
> > In MS Word, this is exactly how it works. You can click the current language
> > in the status bar to switch to another one.
> 
> ...and switching the language from status bar in MS Word works like a
> charme, very convenient because you don't have to search for it between all
> the other options. So we could just try the same... :)

I really like that idea, allowing users to change the language from that statusbar widget also gives us a better excuse for hijacking the holy status bar at all...

We could also do this in a followup bug, but from coding the demo I'd think it might be simple enough to just do it while we are here.

Features & benefits (as seen in the XUL demo):
* visually de-emphasise the permanent display of current language in status bar (ux-minimalism; ux-visual-hierarchy)
* buttonize and tooltip on hover: "Choose language for spell check" (ux-discovery)
* Language selection menu on left or right click, similar to MS Word (ux-efficiency; external ux-consistency)
* Re-using the existing language menu of the spellcheck dual button (no code duplication)

Feedback welcome :)

Comment 216

[Thomas D. (:thomas8)]

Comment on attachment 8648711 [details]
Demo2.xul: statusbar language selector (like MS Word)

Please play with the Spell Check Language Selector on Status Bar...

Comment 217

[Jorg K (CEST = GMT+2)]

Richard, do you want to take up any of these suggestions?
text color: darkgrey. That would mean changes to three CSS files (Linux, Mac, Windows).
Or just style="color:darkgrey; in the panel definition?

Do you want a popup menu on the status bar (!)? The security settings can't be changed from their icons.

As we've seen with two buttons with menu lists, we would need to maintain two lists of languages with two IDs in the code. This solution would be far more complex.

> Features & benefits (as seen in the XUL demo).
Sorry, I'm new here, what do I do with the XUL file?

Comment 218

[Richard Marti (:Paenglab)]

Comment on attachment 8648711 [details]
Demo2.xul: statusbar language selector (like MS Word)

f- because I see no statusbar with your example and please use a new bug for this. This bug is long enough and we shouldn't add new features in this stage as it was hard enough to come to this state. This doesn't mean I'm against this.

This functionality is what dictionary switcher is doing I mentioned in comment 174 does.

Comment 219

[Richard Marti (:Paenglab)]

(In reply to Jorg K - let's discuss some more so it never gets done ;-) from comment #217)
> Richard, do you want to take up any of these suggestions?
> text color: darkgrey. That would mean changes to three CSS files (Linux,
> Mac, Windows).
> Or just style="color:darkgrey; in the panel definition?

We should leave it like it is. Like it is now on bottom right it doesn't distract the user.

> Do you want a popup menu on the status bar (!)? The security settings can't
> be changed from their icons.
> 
> As we've seen with two buttons with menu lists, we would need to maintain
> two lists of languages with two IDs in the code. This solution would be far
> more complex.

This should be made in a new bug as this one is long enough with a lot of changes.

> > Features & benefits (as seen in the XUL demo).
> Sorry, I'm new here, what do I do with the XUL file?

You'd need to install in FX the Remote XUL manager from AMO and then add a permission for the site where the XUL is running.

Comment 220

[Thomas D. (:thomas8)]

(In reply to Richard Marti (:Paenglab) from comment #219)
> (In reply to Jorg K - let's discuss some more so it never gets done ;-) from
> comment #217)
> > Richard, do you want to take up any of these suggestions?
> > text color: darkgrey. That would mean changes to three CSS files (Linux,
> > Mac, Windows).

Yes, improvements always need work...

> > Or just style="color:darkgrey; in the panel definition?

No, we don't hard-code any theme styles in the XUL. You'd use something like "disabled text" color as offered by the theme, so when the theme uses different colors like dark background, they can change it according to their color schemes.

> We should leave it like it is. Like it is now on bottom right it doesn't
> distract the user.

Wait until Blake sees that we've added *anything* to the status bar... ;)
He didn't even want to add a zoom display/control which imo is more important and useful than the language widget...

Anything which is permanently visible is a distraction from the main task at hand, composing, especially things which are needed only rarely (like language). It's visual noise because it needs to be mentally filtered out as "not relevant for what I'm doing" every time you look at that corner. It's a general trend to ux-minimalize things as we're trying to reduce the overall visual complexity of the interface; that's why button borders, menus, toolbars, status bars have been removed or tamed in many applications over time. But well, it's polish so it could also be handled in another bug.

> > Do you want a popup menu on the status bar (!)? The security settings can't
> > be changed from their icons.

> > As we've seen with two buttons with menu lists, we would need to maintain
> > two lists of languages with two IDs in the code. This solution would be far
> > more complex.

Repeating false statements doesn't make them right.
Please try it: Leave the original popup with ID "languageMenuList" inside the combined spellcheck toolbar button, and use showPopup to link it up from the onclick event of the status bar button as I've demonstrated in demo2, attachment 8648711 [details]. A *single* popup with a single ID is enough to be shared in several places - easy and straightforward.
Even in the other demo1, attachment 8648489 [details], it could be made to work in the same way, but the actual demo1 has the popup in an external popupset, which would require a few extra lines to get it working on the dual button (iow, demo1 is incomplete).

> This should be made in a new bug as this one is long enough with a lot of
> changes.

I'm fine with trying my idea in a new bug.

Comment 221

[Jorg K (CEST = GMT+2)]

(In reply to Thomas D. from comment #220)
> Repeating false statements doesn't make them right.
Indeed.

Two different elements in the UI need two different IDs to address them in JavaScript code.
The JavaScript call is "document.getElementById('languageMenuList');" in this case.

To investigate your proposal where the old "Spelling" button was kept around for configuration purposes, I had "Language" button and (old) "Spelling" button both defined with:
<menupopup id="languageMenuList" oncommand="ChangeLanguage(event);"
  onpopupshowing="OnShowDictionaryMenu(event.target);"/>
Twice the same ID "languageMenuList". Result: the popup only showed on the "Language" button, not the old "Spelling" button, since the element defined first was addressed. That's when I stopped.

I'm happy to stand corrected when you provide the next patch in what will be "your" new bug ;-)
I for sure won't touch any UX bug ever again with a 10 foot pole (as the say in Australia).

Comment 222

[:aceman]

(In reply to Jorg K - let's discuss some more so it never gets done ;-) from comment #221)
> To investigate your proposal where the old "Spelling" button was kept around
> for configuration purposes, I had "Language" button and (old) "Spelling"
> button both defined with:
> <menupopup id="languageMenuList" oncommand="ChangeLanguage(event);"
>   onpopupshowing="OnShowDictionaryMenu(event.target);"/>
> Twice the same ID "languageMenuList". Result: the popup only showed on the
> "Language" button, not the old "Spelling" button, since the element defined
> first was addressed. That's when I stopped.

Well, you would define it similarly to this:
<menupopup id="languageMenuList" oncommand="ChangeLanguage(event);"
  onpopupshowing="OnShowDictionaryMenu(event.target);"/>
<menulist id="id1" onclick="document.getElementByID('languageMenuList').openPopup(...)">
<menulist id="id2" onclick="document.getElementByID('languageMenuList').openPopup(...)">

What Thomas says should technically be possible, I just am not sure how much it is used throughout TB (and we do have some duplicated widgets) and whether this pattern is accessible enough and liked.

Comment 223

[Thomas D. (:thomas8)]

Attachment: Demo3.xul: Sharing a single, dynamically populated <menupopup> between different controls

(In reply to Jorg K - let's discuss some more so it never gets done ;-) from comment #221)
> (In reply to Thomas D. from comment #220)
> > Repeating false statements doesn't make them right.
> Indeed.
> 
> Two different elements in the UI need two different IDs to address them in
> JavaScript code.
> The JavaScript call is "document.getElementById('languageMenuList');" in
> this case.

That's trivial. However, as I said, we don't need two different menupopups for sharing the same menupopup and showing it wherever needed, even from clicking the dropdown part of a <button type="menu-popup"> (while not showing it for the main part of that button).

> To investigate your proposal where the old "Spelling" button was kept around
> for configuration purposes, I had "Language" button and (old) "Spelling"
> button both defined with:
> <menupopup id="languageMenuList" oncommand="ChangeLanguage(event);"
>   onpopupshowing="OnShowDictionaryMenu(event.target);"/>
> Twice the same ID "languageMenuList". Result: the popup only showed on the
> "Language" button, not the old "Spelling" button, since the element defined
> first was addressed. That's when I stopped.

Sure. You can't have two elements with the same ID (and I didn't suggest that).

> I'm happy to stand corrected when you provide the next patch in what will be
> "your" new bug ;-)

Please stand corrected:

As shown in this XUL demo3, it's actually very simple to hook up the same menupopup with any control:

* unique {menupopup} defined centrally in {popupset} and dynamically populated with {menuitems}
* for simple controls like {toolbarbutton type="menu"}, just use popup="popupID" attribute (easy as pie, and soooo neat!)
* for tricky controls like the dropdown part of {toolbarbutton type="menu-button"}, use the popup's showPopup() method (in a 3-line helper function)
* for special positioning of the popup (if needed), use optional 'aPosition' parameter

So for the simple cases it's actually *less* and *cleaner* code, and for the hardest case (split menu button), it's a 3-line helper function. Easy. That's neither "far more complex" nor "maintenance nightmare". Some of Aceman's caveats of comment 222 might still apply.

Comment 224

[Magnus Melin [:mkmelin]]

Comment on attachment 8648685 [details] [diff] [review]
Proposed patch: Spelling button always enabled, status info if more than one dictionary installed (v4)

Review of attachment 8648685 [details] [diff] [review]:
-----------------------------------------------------------------

Alright, lets get this in! r=mkmelin

::: mail/components/compose/content/MsgComposeCommands.js
@@ +719,5 @@
> +          window.openDialog("chrome://editor/content/EdSpellCheck.xul",
> +                            "_blank", "dialog,close,titlebar,modal,resizable",
> +                            false, skipBlockQuotes, true);
> +        }
> +        catch(ex) {}

the try-catch looks superflous (though probably copied), so please remove that

Comment 225

[Jorg K (CEST = GMT+2)]

Attachment: LAND THIS: Proposed patch: Spelling button always enabled, status info if more than one dictionary installed (v5)

Carrying forward Richard's ui-r+ and Magnus' r+.

> Alright, lets get this in!
Indeed, we're probably all thoroughly sick of it ;-)

Comment 226

[Jorg K (CEST = GMT+2)]

Attachment: LAND THIS: Proposed patch: Spelling button always enabled, status info if more than one dictionary installed (v5a)

Damn, one line incorrectly indented. Here we go again.

Comment 227

[Thomas D. (:thomas8)]

Comment on attachment 8648685 [details] [diff] [review]

> + <statusbarpanel id="languageText" tooltiptext="&languageText.tooltip;"/>

Perhaps we could use a more descriptive ID? Makes clearer code reading in the long run.
There's no consistent pattern, but most IDs for statusbarpanels have the word "status", "statusbar" or  "statusbarpanel" to indicate where they live. Most IDs also hint what kind of control they are, which is helpful for coding.

I suggest:
<statusbarpanel id="languageStatusbarPanel" tooltiptext="&languageStatusbarPanel.tooltip;"/>

Accordingly, we could then make it

> let languageText = document.getElementById("languageText");
let languageStatusbarPanel = document.getElementById("languageStatusbarPanel");

> languageText.collapsed = true;
languageStatusbarPanel.collapsed = true;

etc.

...which imo is more explicit hence clearer.

Comment 228

[Jorg K (CEST = GMT+2)]

(In reply to Thomas D. from comment #227)
> I suggest:
> <statusbarpanel id="languageStatusbarPanel"
> tooltiptext="&languageStatusbarPanel.tooltip;"/>

Hmm, if anything,
<statusbarpanel id="statusLanguageText" tooltiptext="&statusLanguageText.tooltip;"/>
instead of
<statusbarpanel id="languageText" tooltiptext="&languageText.tooltip;"/>

Magnus?

Comment 229

[Jorg K (CEST = GMT+2)]

Let's wait with the checkin until we get Magnus' answer.

Comment 230

[Magnus Melin [:mkmelin]]

statusLanguageText or keeping it as it is work for me.

Comment 231

[Thomas D. (:thomas8)]

statusLanguageText is much better than just LanguageText, although it doesn't indicate the type of control as most IDs do (cmd_xxx, key_xxx, languageMenuList,...). It's just that I think LanguageText is too generic so it's hard to tell where that LanguageText resides.

More alternatives:

statusLanguagePanel
languageStatusPanel

Comment 232

[Thomas D. (:thomas8)]

Having "status" in the ID will also help to distinguish e.g. if someone still wants to implement the language button we had, or if at some point in the future we'll offer formatting different parts of text in different languages. Without "status" in the ID, the reference of our thing here will not be as clear as it could be.

You had probably chosen "Text" as part of ID because the current content of that statusbarpanel is just "text". However, it's just a statusbarpanel and we can't foretell how its contents might change in the future. Maybe we like it so much that we want to add an iconic button for spell-check, too. Maybe we don't like it and want to reduce it to an icon. Maybe, like Word, we want to put an icon indicating the current state of spell checking (errors found or not). As soon as we add anything to the panel it will stop being just text, so the current ID would no longer be adequate, and we'd have to change it just because we didn't pick a more accurate ID now (including the word "Panel" to indicate the type of control).

Yes, it's trivial. But lots of trivia also contribute to better code.

Comment 233

[Jorg K (CEST = GMT+2)]

Attachment: LAND THIS: Spelling button always enabled, status info if more than one dictionary installed (v5b)

Carrying forward Richard's ui-r+ and Magnus' r+.
Changed IDs to what Magnus approved (comment #230 (!!)).
I hope this finally settles this bug.

Comment 234

[Jorg K (CEST = GMT+2)]

Kent, would you consider inclusion of this in ESR38, if I submitted a patch without the tooltip, so that nothing needs to be translated? It would fit nicely into all the usability improvements we've made for TB38 in the area of spell checking. Also see comment #46.

Comment 235

[Thomas D. (:thomas8)]

(In reply to Jorg K from comment #234)
> consider inclusion of this in ESR38, if I submitted a patch
> without the tooltip, so that nothing needs to be translated? It would fit
> nicely into all the usability improvements we've made for TB38 in the area
> of spell checking. Also see comment #46.

+1. Having this for ESR38 would be really nice (we can add the tooltip later as it's not important), and make everyone including the patch author feel better after all the hard work and discussion.

Comment 236

[Kent James (:rkent)]

I have not been following this bug that closely since there seemed to be plenty of cooks already, but I don't have any objection in principal to including this in current TB 38. But I have been more aggressive about accepting patches than we have been in the past, at some point we may need to have a broader discussion of that philosophy.

Comment 237

[Jorg K (CEST = GMT+2)]

Attachment: Patch for ESR38

Kent, this is the patch to consider for ESR38. It's the same as the other patch, only with the tooltip removed. I hope it applies cleanly.

In general I think your "aggressiveness" has been a complete success. Basically it started with creating the "TB 38 release branch" so we could ship M-C changes now instead of waiting for TB45. Do you have any regrets apart from the fact that this has obviously caused more work?

Comment 238

[Thomas D. (:thomas8)]

(In reply to Jorg K (GMT+2) from comment #237)
> In general I think your "aggressiveness" has been a complete success.
> Basically it started with creating the "TB 38 release branch" so we could
> ship M-C changes now instead of waiting for TB45. Do you have any regrets
> apart from the fact that this has obviously caused more work?

+1, I agree with Jorg. From a user/UX perspective, aggressive bug fixing is definitely a good thing, more so given a past track reccord of slow progress, which I think has made many users and contributors give up when things don't get better for several years. I can't judge the workload involved.

Comment 239

[Kent James (:rkent)]

The question in taking this patch in esr38 is this: are you fixing a bug(problem), or adding a feature? I don't want to cross the line of adding features in dot releases, and this bug seems to be an odd mix of feature and fix. You would not have a hundred comments on UI design if this was just fixing a problem.

Comment 240

[Jorg K (CEST = GMT+2)]

Indeed, a mix of fix and feature. I could strip it down to just the fix of always enabling the button.

Comment 241

[Jorg K (CEST = GMT+2)]

Attachment: Patch for ESR38 (reduced to fixing the "Spelling" button)

Comment 242

[Jorg K (CEST = GMT+2)]

Attachment: LAND THIS: Spelling button always enabled, status info if more than one dictionary installed (v5c)

Carrying forward Richard's ui-r+ and Magnus' r+.

Since this hasn't landed ...
Minor tweak to when the active dictionary gets removed.
This now allows the user in all cases to select a new dictionary.

Magnus, I hope you are OK with this.

Comment 243

[Jorg K (CEST = GMT+2)]

So you don't have to go looking, this is what I added:

 function OnShowDictionaryMenu(aTarget)
 {
   InitLanguageMenu();
   let spellChecker = gSpellChecker.mInlineSpellChecker.spellChecker;
   let curLang = spellChecker.GetCurrentDictionary();
+  if (!curLang || curLang != document.documentElement.getAttribute("lang")) {
+    // Looks like the active dictionary got removed, or something is
+    // inconsistent. In this case do no check anything, so the user can select
+    // another dictionary.
+    return;
+  }
   let language = aTarget.querySelector('[value="' + curLang + '"]');
   if (language)
     language.setAttribute("checked", true);
 }

Comment 244

[Magnus Melin [:mkmelin]]

sure (but "do not" instead of "do no")

Comment 245

[Jorg K (CEST = GMT+2)]

Attachment: LAND THIS: Proposed patch: Spelling button always enabled, status info if more than one dictionary installed (v5d)

Carrying forward Richard's ui-r+ and Magnus' r+.
Fixed typo.

Comment 246

[Jorg K (CEST = GMT+2)]

Attachment: LAND THIS: Proposed patch: Spelling button always enabled, status info if more than one dictionary installed (v5e)

Carrying forward Richard's ui-r+ and Magnus' r+.
doCommand function has no argument.

Comment 247

[:aceman]

Comment on attachment 8648711 [details]
Demo2.xul: statusbar language selector (like MS Word)

I think everything was answered here.

Comment 248

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/1297da85ccdb32143e5b9b4ba514b2000b4271ec
Bug 368915 - Enable spell check in subject line, provide language info in status bar. ui-r=Paenglab, r=mkmelin a=aleth on CLOSED TREE

Comment 249

[aleth [:aleth]]

Thanks! Please add uplift flags as required.

Comment 250

[Jorg K (CEST = GMT+2)]

Not sure if we want to uplift attachment 8650115 [details] [diff] [review].

Comment 251

[:aceman]

Verified in Win nighlty of 2015-09-06.
The context menu of subject and msg body contains "Check spelling" and can be individually toggled. Incorrect words are underlined, in subject and also body.
The spelling button is enabled whether I am in subject or body.
Clicking the button shows the spelling dialog with found problems. But only those from the body. Cycling the found errors finishes parsing the body but never shows problems from subject. I did not find a way to open the dialog with errors from subject. Is that intentional for now?

Comment 252

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #251)
> Is that intentional for now?
Yes.

Spell checking in the subject never worked. Therefore the button was context-sensitive.

I wanted to implement another "language" button, which would always have been available. This proposal was rejected (refer to the long discussion above).

So now the "Spelling" button is always available, but the spell check in the subject still doesn't work.

Comment 253

[Thomas D. (:thomas8)]

(In reply to :aceman from comment #251)
> Cycling the found errors finishes parsing the body but
> never shows problems from subject. I did not find a way to open the dialog
> with errors from subject. Is that intentional for now?

It's better than before because we now allow language selection in subject which is more important.
Spelling errors in subject are more exposed so you are likely to see them even though they are skipped.

However, we should fix this. The UX is not very bad, but it's ridiculous. I'm refusing to believe that it's not possible or extraordinarily hard to somehow iterate over two different spellcheck ranges sequentially. If we can start the spellcheck for one range, what's stopping us to write javascript which starts the next range after that? It just must be possible. If it's not possible, there's something wrong with the way we've implemented the spellcheck, or the implementation of spellchecker itself.

Comment 254

[Thomas D. (:thomas8)]

(In reply to Jorg K (GMT+2) from comment #250)
> Not sure if we want to uplift attachment 8650115 [details] [diff] [review]
> [review].

Why not? It's a worthwhile and no-risk improvement...

Comment 255

[Jorg K (CEST = GMT+2)]

(In reply to Thomas D. from comment #253)
> I'm refusing to believe that it's not possible or extraordinarily hard to
> somehow iterate over two different spellcheck ranges sequentially.
You try it.

> If we can start the spellcheck for one range, what's stopping us ...
Lack of resources who think that this is important. 

> It just must be possible.
Anything is possible.

Comment 256

[Kent James (:rkent)]

For uplifting, unless a patch is critical for the success of Thunderbird 38, it would be best to first uplift to aurora and beta, wait for a beta cycle, then uplift to esr38 if appropriate.

Comment 257

[Wayne Mery (:wsmwk)]

(In reply to Kent James (:rkent) from comment #256)
> For uplifting, unless a patch is critical for the success of Thunderbird 38,
> it would be best to first uplift to aurora and beta, wait for a beta cycle,
> then uplift to esr38 if appropriate.

It's great to see a fix but I think something like this should at the very least ride the full train. Perhaps even wait til next esr. We have plenty of manpower eating important issues to fight without introducing potential additional regressions in 38

Comment 258

[Jorg K (CEST = GMT+2)]

We all agree that the full fix needs to ride the trains.

The "small solution" (attachment 8650115 [details] [diff] [review]) is trivial and won't cause any regressions. We all agree that it's not very important.