Closed Bug 984979 Opened 6 years ago Closed 6 years ago

Australis - Back button's open state doesn't match :active state on Windows 8

Categories

(Firefox :: Theme, defect)

31 Branch
All
Windows 8
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 31
Tracking Status
firefox29 --- verified
firefox30 --- verified
firefox31 --- verified

People

(Reporter: ntim, Assigned: ntim)

References

Details

(Whiteboard: [Australis:P4])

Attachments

(1 file)

On Windows 8, the back button's active state is inconsistent with other button active states.
Blocks: theme-win8
OS: Windows 8.1 → Windows 8
Hardware: x86_64 → All
Whiteboard: [Australis:P-]
I guess this is due to the shape, and by design. Stephen?
Flags: needinfo?(shorlander)
Whiteboard: [Australis:P-] → [Australis:P?]
(In reply to :Gijs Kruitbosch from comment #1)
> I guess this is due to the shape, and by design. Stephen?

It might just be the hover background overriding the active background.
(In reply to Tim Nguyen [:ntim] from comment #2)
> (In reply to :Gijs Kruitbosch from comment #1)
> > I guess this is due to the shape, and by design. Stephen?
> 
> It might just be the hover background overriding the active background.

*for the back button
(In reply to :Gijs Kruitbosch from comment #1)
> I guess this is due to the shape, and by design. Stephen?

I don't think so, the mockup shows a consistent background for all toolbar button active state.
Turns out Mike forgot to add an active state for the back button.
http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser.css#884

A fix would be :
>+#back-button:hover:active > .toolbarbutton-icon
> #back-button[open="true"] > .toolbarbutton-icon {
>    background-color: hsla(210,4%,10%,.12) !important;
>    box-shadow: 0 1px 0 0 hsla(210,80%,20%,.1) inset !important;
> }
Patches welco...hey! :)

If you're not already, have you considered building locally and generating actual patches? It's the fastest way to help get nits like this fixed, and yay open source!

https://developer.mozilla.org/en-US/docs/Simple_Firefox_build

https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Whiteboard: [Australis:P?] → [Australis:P4]
(In reply to Justin Dolske [:Dolske] from comment #6)
> Patches welco...hey! :)
> 
> If you're not already, have you considered building locally and generating
> actual patches? It's the fastest way to help get nits like this fixed, and
> yay open source!
> 
> https://developer.mozilla.org/en-US/docs/Simple_Firefox_build
> 
> https://developer.mozilla.org/en-US/docs/
> Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-
> in_for_me.3F

Ive done the codefirefox videos tutorials already, but I somehow screwed up my configuration. I'm currently looking into github development for this, since I'm more used to the environment.
(In reply to Tim Nguyen [:ntim] from comment #7)
> (In reply to Justin Dolske [:Dolske] from comment #6)
> > Patches welco...hey! :)
> > 
> > If you're not already, have you considered building locally and generating
> > actual patches? It's the fastest way to help get nits like this fixed, and
> > yay open source!
> > 
> > https://developer.mozilla.org/en-US/docs/Simple_Firefox_build
> > 
> > https://developer.mozilla.org/en-US/docs/
> > Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-
> > in_for_me.3F
> 
> Ive done the codefirefox videos tutorials already, but I somehow screwed up
> my configuration. I'm currently looking into github development for this,
> since I'm more used to the environment.

Alternatively, drop by in #introduction on http://irc.mozilla.org/ and we can try and help you?
So this appears to have the correct depressed state if the history menu is open, but not on :active.
Flags: needinfo?(shorlander)
Summary: Australis - Back button has inconsistent :active state on Windows 8 → Australis - Back button's open state doesn't match :active state on Windows 8
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → ntim007
Status: NEW → ASSIGNED
Attached patch bug_984979.patchSplinter Review
My first patch :)
Attachment #8395275 - Flags: review?(mdeboer)
Attachment #8395275 - Flags: review?(mdeboer) → review?(gijskruitbosch+bugs)
Comment on attachment 8395275 [details] [diff] [review]
bug_984979.patch

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

LGTM! Thanks for the patch. :-)
Attachment #8395275 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to :Gijs Kruitbosch from comment #11)
> Comment on attachment 8395275 [details] [diff] [review]
> bug_984979.patch
> 
> Review of attachment 8395275 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> LGTM! Thanks for the patch. :-)

Thanks :) Gonna ask for checkin.
Keywords: checkin-needed
remote:   https://hg.mozilla.org/integration/fx-team/rev/77802de7e213
Keywords: checkin-needed
Whiteboard: [Australis:P4] → [Australis:P4][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/77802de7e213
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P4][fixed-in-fx-team] → [Australis:P4]
Target Milestone: --- → Firefox 31
Comment on attachment 8395275 [details] [diff] [review]
bug_984979.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 960517
User impact if declined: Awkward back button :active state
Testing completed (on m-c, etc.):  on m-c
Risk to taking this patch (and alternatives if risky): None
String or IDL/UUID changes made by this patch: None
Attachment #8395275 - Flags: approval-mozilla-beta?
Attachment #8395275 - Flags: approval-mozilla-aurora?
Attachment #8395275 - Flags: approval-mozilla-beta?
Attachment #8395275 - Flags: approval-mozilla-beta+
Attachment #8395275 - Flags: approval-mozilla-aurora?
Attachment #8395275 - Flags: approval-mozilla-aurora+
Mozilla/5.0 (Windows NT 6.2; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0		
Mozilla/5.0 (Windows NT 6.2; rv:29.0) Gecko/20100101 Firefox/29.0		

Verified issue fixed using Firefox 29 beta 3 (build ID: 20140327113732), latest Aurora (build ID: 20140327004002) and latest Nightly (build ID: 20140327030203).
\o/, belatedly. Thanks for the patch!
You need to log in before you can comment on or make changes to this bug.