1207658 - to/cc drop-down not highlighted when selected

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[waterbug]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_1) AppleWebKit/600.1.25 (KHTML, like Gecko) Version/8.0 Safari/600.1.25

Steps to reproduce:

Create a new message via New or Reply. Type an address in the first To: field and then press Enter. Focus correctly moves to the next address line. Type a second address and then press Shift-Tab.

The focus correctly moves to the second To:/cc: drop-down, but there is no visual indication that it's selected. 

It should be highlighted to inform the user that typed entries will affect this drop-down.

Tabbing/Shift-tabbing to any of the to:/cc: drop-downs has the same symptoms.


Actual results:

To:/cc: drop-downs do not get highlighted when selected with tab/shift-tab.


Expected results:

To:/cc: drop-downs should be highlighted when selected.

Comment 1

[Thomas D. (:thomas8)]

On my TB 38.2.0, Win 8.1, I see dotted border when highlighted.
(Although I'd much prefer white background when highlighted, what about you, waterbug?)

Which TB/OS version you running on?

Comment 2

[waterbug]

I'm running the latest TB 38.2.0 on OS X 10.10.1. 

Here's what it looks like when the focus is on the actual address (note the cursor):

https://dl.dropboxusercontent.com/u/3552590/FocusOnAddress.png

Here's what it looks like when the focus is on the To/CC selector:

https://dl.dropboxusercontent.com/u/3552590/FocusOnToCC.png

The To/CC selector looks pixel-identical to me in both cases.

Thanks!

Comment 3

[Richard Marti (:Paenglab)]

Tabbing on my OS X goes never to the menulists. Is there a setting to enable this?

Comment 4

[Richard Marti (:Paenglab)]

Attachment: focusState.patch

This patch adds a focusring around .aw-menulist when focused.

I also cleaned some code and fixed some aligning.

Comment 5

[aleth [:aleth]]

Comment on attachment 8721656 [details] [diff] [review]
focusState.patch

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

Lgtm, but I'm not a compose peer.

Comment 6

[Richard Marti (:Paenglab)]

But Jörg has no OS X to check this.

Comment 7

[Jorg K (CEST = GMT+2)]

Comment on attachment 8721656 [details] [diff] [review]
focusState.patch

Sorry, no OS X.
(Last time I compiled something on try and downloaded the Mac binary, my virtual Mac OS claimed that the file was corrupt. The virtual machine worked for downloaded FF binaries back in Nov., so no idea what was wrong with the TB build.)

I guess Aleth will have to overstep his powers here ;-) We won't tell anyone. Or you get Philipp.

Comment 8

[aleth [:aleth]]

Comment on attachment 8721656 [details] [diff] [review]
focusState.patch

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

I tested this and it doesn't seem to work consistently. Maybe the expected behaviour isn't clear to me.

E.g. on the screenshot, the second line always has a white background, while the others only do when focused.

I couldn't find a way to focus the dropdown on 'To' with the keyboard using shift-tab at all (as seems to be described in comment 0). Instead, Shift-tab moves to the previous To value line (which might be the expected behaviour, I don't know).

Comment 9

[aleth [:aleth]]

Attachment: Screen Shot.png

Comment 10

[Jorg K (CEST = GMT+2)]

Let's take the case of your screen shot:
To: huhu
To: hihi
To: hoho
These are six fields. On Windows, <tab> and <shift tab> cycle through the fields and there is visual feedback: The To: field gets a dotted outline and the address fields get a light background when focused.

Comment 11

[Richard Marti (:Paenglab)]

(In reply to aleth [:aleth] from comment #8)
> Comment on attachment 8721656 [details] [diff] [review]
> focusState.patch
> 
> Review of attachment 8721656 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I tested this and it doesn't seem to work consistently. Maybe the expected
> behaviour isn't clear to me.
> 
> E.g. on the screenshot, the second line always has a white background, while
> the others only do when focused.

Here it works, maybe you had the mouse pointer over this field.

> I couldn't find a way to focus the dropdown on 'To' with the keyboard using
> shift-tab at all (as seems to be described in comment 0). Instead, Shift-tab
> moves to the previous To value line (which might be the expected behaviour,
> I don't know).

I had also long to search. In System Preferences/Language & Region/Keyboard Preferences.../Shortcuts you can on the bottom change it to 'All controls'. Or simply by pressing 'Ctrl + F7' everywhere.

Comment 12

[Richard Marti (:Paenglab)]

Comment on attachment 8721656 [details] [diff] [review]
focusState.patch

Aleth, please can you try it again with the instruction in comment 11?

Comment 13

[aleth [:aleth]]

(In reply to Richard Marti (:Paenglab) from comment #11)
> > E.g. on the screenshot, the second line always has a white background, while
> > the others only do when focused.
> 
> Here it works, maybe you had the mouse pointer over this field.

I can't reproduce the problem any more after a rebuild, so maybe this was the case.

> I had also long to search. In System Preferences/Language & Region/Keyboard
> Preferences.../Shortcuts you can on the bottom change it to 'All controls'.
> Or simply by pressing 'Ctrl + F7' everywhere.

Thanks, I wasn't aware from the bug description that this had to be turned on to see the effect of the patch.

@jorgk: I also encountered this warning in testing - looks like there should be a boolean check somewhere.
JavaScript error: chrome://messenger/content/messengercompose/MsgComposeCommands.js, line 3575: TypeError: gMsgCompose is null

Comment 14

[Jorg K (CEST = GMT+2)]

(In reply to aleth [:aleth] from comment #13)
> @jorgk: I also encountered this warning in testing - looks like there should
> be a boolean check somewhere.
> JavaScript error:
> chrome://messenger/content/messengercompose/MsgComposeCommands.js, line
> 3575: TypeError: gMsgCompose is null

You mean this line:
if (gContentChanged || gMsgCompose.bodyModified || gAutoSaveKickedIn)

Luckily that's nothing I've touched since being with the company ;-)

Any particular STR? This happens when you close a message composition window. So I closed a few and nothing happened.

Comment 15

[Richard Marti (:Paenglab)]

https://hg.mozilla.org/comm-central/rev/131bd1aea9ee

Comment 16

[Richard Marti (:Paenglab)]

Comment on attachment 8721656 [details] [diff] [review]
focusState.patch

[Approval Request Comment]
User impact if declined: no visible focus on OS X with enabled option
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low, small changes

Comment 17

[Jorg K (CEST = GMT+2)]

Aurora (TB 46):
https://hg.mozilla.org/releases/comm-aurora/rev/c9bebdf6e63c

Comment 18

[aleth [:aleth]]

See bug 1250605.

Comment 19

[Kent James (:rkent)]

Comment on attachment 8721656 [details] [diff] [review]
focusState.patch

http://hg.mozilla.org/releases/comm-esr45/rev/04ea6b38f025

Comment 20

[Jorg K (CEST = GMT+2)]

Comment on attachment 8721656 [details] [diff] [review]
focusState.patch

[Triage Comment]