Closed Bug 1358136 Opened 4 years ago Closed 4 years ago

No hover/mouseover state in high contrast theme

Categories

(Firefox :: Address Bar, defect, P2)

55 Branch
All
Windows
defect

Tracking

()

VERIFIED FIXED
Firefox 55
Tracking Status
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- verified

People

(Reporter: simona.marcu, Assigned: adw)

References

Details

(Whiteboard: [fxsearch])

Attachments

(5 files)

Attached video hover in dark theme.mp4
[Affected versions]:
- Nightly 55.0a1

[Affected platforms]:
- Windows 10, Windows 7

[Steps to reproduce]:
1. Launch Firefox 
2. Type anything in the Awesome Bar
3. Hover your mouse over the autocomplete dropdown and over the one-off buttons
4. Type anything in the Search bar
5. Hover your mouse over the search suggestions results and one-off buttons.

[Expected result]:
- In step 3 and 5 - hover/mouseover state is properly seen.

[Actual result]:
- A very discrete shadow can be seen when hovering the mouse over the one-off buttons but the hover/mouseover state is missing completely on the autocomplete dropdown and on the search suggestions results. Please see the screencast for more details.
Priority: -- → P2
Whiteboard: [fxsearch]
Assignee: nobody → adw
Status: NEW → ASSIGNED
--arrowpanel-dimmed-even-further looks pretty good.  I'll post some screenshots.
Attached image bug1358136.png
The first image is the three dark high-contrast modes -- they all look alike in this case.  The second image is the white mode.  The Yahoo button is moused-over.
I don't think this qualifies as a blocker for the feature, but I'm happy to land it ASAP since we have a working patch.
No longer blocks: 1337003
Blocks: 1180944
Comment on attachment 8861249 [details]
Bug 1358136 - No hover/mouseover state in high contrast theme.

https://reviewboard.mozilla.org/r/133204/#review136688

::: browser/themes/windows/searchbar.css:235
(Diff revision 1)
>  }
>  
> +@media (-moz-windows-default-theme) {
> +  .searchbar-engine-one-off-item:not([selected]):not(.dummy):hover,
> +  .addengine-item:hover {
> +    background-color: hsla(0, 0%, 0%, 0.06);

--arrowpanel-dimmed, --arrowpanel-dimmed-further and --arrowpanel-dimmed-even-further were designed to work with default and non-default themes. Please use them without hardcoding a color for the default theme.
And please fix this on Linux too. On Mac, hardcoding hsla(0, 0%, 0%, 0.06); will work but I don't think there's a reason to prefer this over the CSS variables.
Blocks: 1358676
Comment on attachment 8861249 [details]
Bug 1358136 - No hover/mouseover state in high contrast theme.

Looks like Dão already reviewed this in comment 5 and 6, and I think you can flag him on the next version.
Attachment #8861249 - Flags: review?(florian)
(In reply to Dão Gottwald [::dao] from comment #6)
> will work but I don't think there's a reason to prefer this over the CSS
> variables.

Well, it's what Stephen specified...
The difference is unlikely to be intentional and shouldn't really be visible to the naked eye. We don't want to go back to having similar but slightly different hardcoded colors scattered across themes; we need to try to bring UX specs in line with how our themes are structured, at least where that structure is intentional to prevent our code base from becoming a chaotic mess with user-visible declining quality (see this bug for instance).
If you're curious about the history of this, I introduced these variables in bug 1301758 to replace hardcoded colors that lacked consistency and generally didn't work with dark OS themes (bug 1302355).
Thanks for explaining, I appreciate it. :-)
--arrowpanel-dimmed-further is closest to Stephen's spec.
.search-setting-button also already uses --arrowpanel-dimmed-further, so it's good that the other buttons match that.
Comment on attachment 8861249 [details]
Bug 1358136 - No hover/mouseover state in high contrast theme.

https://reviewboard.mozilla.org/r/133204/#review137938
Attachment #8861249 - Flags: review?(dao+bmo) → review+
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/214724a4154b
No hover/mouseover state in high contrast theme. r=dao
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0b2bc96e1040
No hover/mouseover state in high contrast theme. r=dao
https://hg.mozilla.org/mozilla-central/rev/0b2bc96e1040
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
When trying to verify this issue I observed the following:

1. In the Awesome Bar drop down and in the Search Bar drop-down - the mouse over state has the same color as the one-offs bar.

2. In the Awesome Bar drop down and in the Search Bar drop-down - when hovering the autocomplete result and the one off button the mouseover state color is different. 

3. In the search bar - the color of the mouse state for Change Search Settings doesn't match the color of mouse state of the one-offs or of the search suggestions.

Drew, are this intended by design?
Flags: needinfo?(adw)
Attached video mouseoverState.mp4
Please see the screencast for more details.
Hi Simona, thanks for the questions and screencast.  In summary, I think you're seeing the intended behavior.  Details below.

(In reply to Simona B [:simonab ] from comment #19)
> 1. In the Awesome Bar drop down and in the Search Bar drop-down - the mouse
> over state has the same color as the one-offs bar.

Yes, they both use `background-color: var(--arrowpanel-dimmed)`:

https://dxr.mozilla.org/mozilla-central/rev/85e5d15c31691c89b82d6068c26260416493071f/browser/themes/windows/browser.css#1086

https://dxr.mozilla.org/mozilla-central/rev/85e5d15c31691c89b82d6068c26260416493071f/browser/themes/windows/searchbar.css#172

> 2. In the Awesome Bar drop down and in the Search Bar drop-down - when
> hovering the autocomplete result and the one off button the mouseover state
> color is different.

Yes, they use different colors.  The urlbar result hover uses `background-color: var(--arrowpanel-dimmed)` (as mentioned above), and the one-off hover color uses `background-color: var(--arrowpanel-dimmed-further)`:

https://dxr.mozilla.org/mozilla-central/rev/85e5d15c31691c89b82d6068c26260416493071f/browser/themes/windows/searchbar.css#217

> 3. In the search bar - the color of the mouse state for Change Search
> Settings doesn't match the color of mouse state of the one-offs or of the
> search suggestions.

It's hard to tell from the screencast, but the settings button and the one-off buttons should have the same background color on hover.  They both use `background-color: var(--arrowpanel-dimmed-further)`:

https://dxr.mozilla.org/mozilla-central/rev/85e5d15c31691c89b82d6068c26260416493071f/browser/themes/windows/searchbar.css#337

They do look the same to me in your screencast.  Are they actually different?
Flags: needinfo?(adw)
Attached image Screenshoot1.png
(In reply to Drew Willcoxon :adw from comment #21)
> It's hard to tell from the screencast, but the settings button and the
> one-off buttons should have the same background color on hover.  They both
> use `background-color: var(--arrowpanel-dimmed-further)`:
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> 85e5d15c31691c89b82d6068c26260416493071f/browser/themes/windows/searchbar.
> css#337
> 
> They do look the same to me in your screencast.  Are they actually different?

Drew, please see the screenshot. The RGB of the two elements (one-off button while being moused hovered and Change Search Settings button while being mouse hovered) is different. I used the add-on ColorPicker and the RGB for the one-off button is rgb(126,126,126) while the RGB for the Change Search Settings button is rgb(92,92,92).
Flags: needinfo?(adw)
Ah I see, thanks for the screenshot.  I don't know what's causing the difference, but I'll file a new bug for it.  It'll be low priority though and not a blocker for release.
Flags: needinfo?(adw)
Depends on: 1366872
Verified fixed on the latest Nightly 55.0a1 (Build ID: 20170523030206) - the initial issue is no longer reproducible. Verification was done on Windows 7 x64 and Windows 10 x 64. 

Considering bug 1366872 is tracking the 3rd issue mentioned in Comment 19, I'm setting the status of this issue to Verified Fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.