1000744 - Fullscreen button no longer shows the depressed state in the menu panel when in fullscreen

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Matthew N. [:MattN]]

Attachment: Screenshot in fullscreen mode on 10.9

This worked some months ago with Australis so it seems like this regressed. It seems like the more specific rules are preventing the change from appearing. The button shows the proper state in toolbars.

This seems to affect at least Windows and OS X.

Comment 1

[Mike de Boer [:mikedeboer]]

This was regressed by bug 979378.

Taking & fixing this.

Comment 2

[Mike de Boer [:mikedeboer]]

Attachment: Patch v1: re-introduce a checked state for panel-menu buttons

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 8411644 [details] [diff] [review]
Patch v1: re-introduce a checked state for panel-menu buttons

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

Don't forget to update the reviewer.

Thanks for jumping on this so quickly and please request uplift.

::: browser/themes/shared/customizableui/panelUIOverlay.inc.css
@@ +727,5 @@
>  .toolbaritem-combined-buttons@inAnyPanel@@buttonStateHover@ {
>    border-color: hsla(210,4%,10%,.11);
>  }
>  
> +panelview .toolbarbutton-1:-moz-any(@buttonStateActive@,[checked=true]),

OK, my understanding is that it's fine to use this as the right-most selector here for perf.[1] since it's attached to the toolbarbutton-1 class.

[1] https://developer.mozilla.org/en-US/docs/Web/CSS/:any#Issues_with_performance_and_specificity

Comment 4

[Mike de Boer [:mikedeboer]]

Thanks!

Pushed as https://hg.mozilla.org/integration/fx-team/rev/0bfe1f94e874

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0bfe1f94e874

Comment 6

[Mike de Boer [:mikedeboer]]

Comment on attachment 8411644 [details] [diff] [review]
Patch v1: re-introduce a checked state for panel-menu buttons

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis, bug 979378.
User impact if declined: When in fullscreen mode, the Fullscreen button in the menu-panel does not show this state, whereas before it did. This patch solves this regression and makes sure the Fullscreen button show the 'down' state when in fullscreen mode.
Testing completed (on m-c, etc.): landed on m-c.
Risk to taking this patch (and alternatives if risky): minor.
String or IDL/UUID changes made by this patch: n/a.

Comment 7

[Mike de Boer [:mikedeboer]]

Please note that approval was granted for Fx 30, thus needs to be landed on beta, if I'm correct.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

FWIW, last I heard, Australis devs are still handling their own uplifts.

Comment 9

[Mike de Boer [:mikedeboer]]

Pushed to beta as: https://hg.mozilla.org/releases/mozilla-beta/rev/6c2ea428e1f8

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/6c2ea428e1f8

Comment 11

[Andrei Vaida [:avaida]]

Verified fixed on Firefox 31 Beta 4 (Build ID: 20140623175014) using: Windows 7 64-bit, Mac OS X 10.9.2 and Ubuntu 14.04 LTS 32-bit.