Closed Bug 1207658 Opened 6 years ago Closed 5 years ago

to/cc drop-down not highlighted when selected

Categories

(Thunderbird :: Message Compose Window, defect)

38 Branch
defect
Not set
normal

Tracking

(thunderbird45 fixed, thunderbird46 fixed, thunderbird47 fixed)

RESOLVED FIXED
Thunderbird 47.0
Tracking Status
thunderbird45 --- fixed
thunderbird46 --- fixed
thunderbird47 --- fixed

People

(Reporter: steven, Assigned: Paenglab)

Details

Attachments

(2 files)

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.
Component: Untriaged → Message Compose Window
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?
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!
Tabbing on my OS X goes never to the menulists. Is there a setting to enable this?
Attached patch focusState.patchSplinter Review
This patch adds a focusring around .aw-menulist when focused.

I also cleaned some code and fixed some aligning.
Assignee: nobody → richard.marti
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8721656 - Flags: review?(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.
Attachment #8721656 - Flags: review?(aleth) → review?(mozilla)
But Jörg has no OS X to check this.
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.
Attachment #8721656 - Flags: review?(mozilla)
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).
Attachment #8721656 - Flags: review-
Attached image Screen Shot.png
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.
(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 on attachment 8721656 [details] [diff] [review]
focusState.patch

Aleth, please can you try it again with the instruction in comment 11?
Attachment #8721656 - Flags: review- → review?(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
Attachment #8721656 - Flags: review?(aleth) → review+
Keywords: checkin-needed
(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.
Flags: needinfo?(aleth)
https://hg.mozilla.org/comm-central/rev/131bd1aea9ee
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 47.0
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
Attachment #8721656 - Flags: approval-comm-beta?
Attachment #8721656 - Flags: approval-comm-aurora?
Attachment #8721656 - Flags: approval-comm-aurora? → approval-comm-aurora+
See bug 1250605.
Flags: needinfo?(aleth)
Comment on attachment 8721656 [details] [diff] [review]
focusState.patch

http://hg.mozilla.org/releases/comm-esr45/rev/04ea6b38f025
Attachment #8721656 - Flags: approval-comm-beta? → approval-comm-beta+
Comment on attachment 8721656 [details] [diff] [review]
focusState.patch

[Triage Comment]
Attachment #8721656 - Flags: approval-comm-beta+ → approval-comm-esr45+
You need to log in before you can comment on or make changes to this bug.