Closed Bug 1466486 Opened 6 years ago Closed 6 years ago

"Tab" key doesn't highlight some of the buttons from DevTools

Categories

(DevTools :: General, defect, P1)

61 Branch
defect

Tracking

(firefox-esr52 unaffected, firefox-esr60 unaffected, firefox60 unaffected, firefox61 wontfix, firefox62 verified)

VERIFIED FIXED
Firefox 62
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox60 --- unaffected
firefox61 --- wontfix
firefox62 --- verified

People

(Reporter: obotisan, Assigned: mantaroh)

References

Details

(Keywords: regression)

Attachments

(3 files, 2 obsolete files)

[Affected versions]:
- Latest Nightly 62.0a1
- Beta 61.0b10


[Affected platforms]:
- Windiws 10 x64
- Ubuntu 16.04 x64
- macOS 10.13

[Steps to reproduce]:
1. Open a blank page (write about:blank in the URL bar)
2. Press Crtl+Shift+K
3. Click on the button "Toggle filter bar" to activate the filter bar.
4. Press "Tab" from the keyboard and observe the buttons: "Toggle filter bar", "Errors" "Warnings" "Log" etc.

[Expected result]:
- The buttons are highlighted and you know which one is in focus.

[Actual result]:
- The buttons are not highlighted making it impossible to see which one is in focus. 

[Regression range]:
- Mozregression concluded that Bug 1444793 - Make focus style of tool tab to be same as hover style - might be the cause of the problem.
- last good build: 2018-04-18
- first bad build: 2018-04-19

[Additional notes]:
- The buttons actually work if you press "Enter", they are just not highlighted and you don't know on which one the focus is on. 
- This issue affects other sections, not only the "Console" one. Almost every section is affected.  
- The issue is reproducible even if the theme is set to dark.
Blocks: 1444793
Flags: needinfo?(mantaroh)
Version: Trunk → 61 Branch
Attached image devtools-button.png (obsolete) —
Thank you for submitting this bug.

This is regression of bug 1444703. I removed the outline focus style from devtools button. However, the following buttons which related with selection override the devtools style, so we should these buttons has another focus style.

 * Console filter bar's button
 * Network filter bar's button

Hi, Victoria,
I forgot take the focus style of overrided button into considering.(e.g. the filter button which overrided default devtools button.)
I think that we need to apply previous focusring style to these button. i.e. these button has the outline(1px dotted outline).
What do you think about this focus style? (An attachment is an image of this style)
Flags: needinfo?(mantaroh) → needinfo?(victoria)
There is a second bug in which visual changes will be happening to these buttons https://bugzilla.mozilla.org/show_bug.cgi?id=1458093

If this needs to happen before that bug is done, for now we could use the hover style for keyboard focus. The blue buttons don't have a hover style at the moment but we could use Blue 60 background color. (I'm trying to phase out the the dotted-line style.)
Flags: needinfo?(victoria)
Yes, we probably shouldn't wait for bug 1458093 since this bug will likely need to be uplifted to 61.
Attached image focus-selection.png (obsolete) —
(In reply to Victoria Wang [:victoria] from comment #2)
> There is a second bug in which visual changes will be happening to these
> buttons https://bugzilla.mozilla.org/show_bug.cgi?id=1458093
> 
> If this needs to happen before that bug is done, for now we could use the
> hover style for keyboard focus. The blue buttons don't have a hover style at
> the moment but we could use Blue 60 background color. (I'm trying to phase
> out the the dotted-line style.)

Thanks! Victoria,

I want to fix this before bug 1458093 is finished because this is regression and I want to uplift this to the beta.
I will use the hover style to this selection buttons, then we need to 2 styles, hover style of unchecked button and checked button(i.e. grey button and blue button).

This is a wip patch, and an attachment is appearance image:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4f653a16ae956d4fda93a95fb0fa599e2f7050b5
(macOS Build Image:https://queue.taskcluster.net/v1/task/S5hq4LdUTKG_dafCBAxlVQ/runs/0/artifacts/public/build/target.dmg)


Victoria,
Could you please confirm this style? I think it might be hard to notice a little.
Attachment #8983240 - Attachment is obsolete: true
Flags: needinfo?(victoria)
Thanks for this screenshot Mantaroh - I see that I forgot about dark mode, and this light blue hover color for the light mode un-selected is not what I'm seeing in my Nightly. I wasn't able to work on this today but will get back to you later this week.
Here is what I would suggest to make the focus state a bit more clear:

Dark
Focused unchecked - background color: Gray 60, text color: Gray 30
Focused checked - this is fine

Light
Focused unchecked - instead of var(--theme-selection-background-hover), use var(--toolbarbutton-hover-background)
Focused checked - this is fine

Attached a screenshot of how the different colors look together.

Thank you!
Flags: needinfo?(victoria)
Attached image focus.png
(In reply to Victoria Wang [:victoria] from comment #6)
> Created attachment 8984297 [details]
> Screen Shot 2018-06-07 at 2.21.24 PM.png
> 
> Here is what I would suggest to make the focus state a bit more clear:
> 
> Dark
> Focused unchecked - background color: Gray 60, text color: Gray 30
> Focused checked - this is fine
> 
> Light
> Focused unchecked - instead of var(--theme-selection-background-hover), use
> var(--toolbarbutton-hover-background)
> Focused checked - this is fine
> 
> Attached a screenshot of how the different colors look together.
> 
> Thank you!

Thank you so much, Victoria,

I changed the color style of focused selection button.

WIP object: https://queue.taskcluster.net/v1/task/dCVNZ8EVStOB6omOeMWP4A/runs/0/artifacts/public/build/target.dmg
Attachment #8983655 - Attachment is obsolete: true
Product: Firefox → DevTools
Assignee: nobody → mantaroh
Status: NEW → ASSIGNED
Priority: -- → P1
Comment on attachment 8985400 [details]
Bug 1466486 - Apply focus style to devtools selection button.

https://reviewboard.mozilla.org/r/251010/#review257286

Technically, this looks good to me.
I feel there's very little difference between checked-unfocused and checked-focused in light mode though, but I'll defer that to Victoria.

::: commit-message-0344a:4
(Diff revision 1)
> +Bug 1466486 - Apply focus style to devtools selection button. r?nchevobbe
> +
> +The bug 1444793 introduced the focus devtools button style instead of using
> +the outline style. However, devtools selection button didn't bellow this style.

I'm not sure what "bellow" means in this sentence

::: devtools/client/themes/common.css:390
(Diff revision 1)
> +.devtools-button:not(:empty):not(.checked):not(:disabled):focus,
> +.devtools-toolbarbutton:not(:-moz-any([checked=true],[disabled]))[label]:focus {

Those rules are a bit dense to read, so maybe a comment explaining what we are targetting would help here ?
Attachment #8985400 - Flags: review?(nchevobbe) → review+
Comment on attachment 8985400 [details]
Bug 1466486 - Apply focus style to devtools selection button.

https://reviewboard.mozilla.org/r/251010/#review257286

Thank you so much, Nicolas.

> I'm not sure what "bellow" means in this sentence

I addressed this.

> Those rules are a bit dense to read, so maybe a comment explaining what we are targetting would help here ?

There are two styles block which is unchecked button and checked button. I added a comment of these block.
Pushed by mantaroh@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d1379e27688e
Apply focus style to devtools selection button. r=nchevobbe
https://hg.mozilla.org/mozilla-central/rev/d1379e27688e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Please confirm that this is fixed on Nightly.
Flags: qe-verify+
Flags: needinfo?(oana.botisan)
I verified the fix on the latest Nightly 62.0a1 using Windows 10 x64, Ubuntu 16.04 x64 and macOS 10.13. The bug is not reproducing anymore. However, using the light theme, the difference of color is a bit harder to spot. 

Also, there is a moment when you don't know where the focus is. After is passes the "Filter output" field, you have to press two times on "Caps Lock" so that the focus is on "Errors".  I don't think there is something in focus in that moment. Is that a known issue or is it expected behaviour?
Flags: needinfo?(oana.botisan)
Light Focused checked: Yes, it is a bit subtle - how about we change this background color to #0051bd? (It's between Blue 60 and 70 so no standard color token)

Dark Focused unchecked: Change text color to Grey 40 to make it more readable (different from what I posted before)

Sorry for the delay on this; thank you Mantaroh!
Given that we're about to ship 61 and this is just a small visual glitch, I think we can just let this ride the trains.
I have reproduced this issue using Firefox 62.0a1 (2018.06.04) on Win 8.1 x64.
I can confirm this issue is fixed, I verified using Fx latest Nightly 62.0a1 on Win 8.1 x64, Mac OS X 10.12 and Ubuntu 14.04 x64.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: