Closed
Bug 984979
Opened 11 years ago
Closed 11 years ago
Australis - Back button's open state doesn't match :active state on Windows 8
Categories
(Firefox :: Theme, defect)
Tracking
()
VERIFIED
FIXED
Firefox 31
People
(Reporter: ntim, Assigned: ntim)
References
Details
(Whiteboard: [Australis:P4])
Attachments
(1 file)
985 bytes,
patch
|
Gijs
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
On Windows 8, the back button's active state is inconsistent with other button active states.
Assignee | ||
Updated•11 years ago
|
Comment 1•11 years ago
|
||
I guess this is due to the shape, and by design. Stephen?
Flags: needinfo?(shorlander)
Whiteboard: [Australis:P-] → [Australis:P?]
Assignee | ||
Comment 2•11 years ago
|
||
(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.
Assignee | ||
Comment 3•11 years ago
|
||
(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
Assignee | ||
Comment 4•11 years ago
|
||
(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.
Assignee | ||
Comment 5•11 years ago
|
||
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;
> }
Comment 6•11 years ago
|
||
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]
Assignee | ||
Comment 7•11 years ago
|
||
(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.
Comment 8•11 years ago
|
||
(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?
Comment 9•11 years ago
|
||
So this appears to have the correct depressed state if the history menu is open, but not on :active.
Flags: needinfo?(shorlander)
Updated•11 years ago
|
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
Updated•11 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → ntim007
Status: NEW → ASSIGNED
Assignee | ||
Updated•11 years ago
|
Attachment #8395275 -
Flags: review?(mdeboer) → review?(gijskruitbosch+bugs)
Comment 11•11 years ago
|
||
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+
Assignee | ||
Comment 12•11 years ago
|
||
(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
Comment 13•11 years ago
|
||
status-firefox29:
--- → affected
status-firefox30:
--- → affected
Keywords: checkin-needed
Whiteboard: [Australis:P4] → [Australis:P4][fixed-in-fx-team]
Comment 14•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P4][fixed-in-fx-team] → [Australis:P4]
Target Milestone: --- → Firefox 31
Assignee | ||
Comment 15•11 years ago
|
||
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?
Updated•11 years ago
|
status-firefox31:
--- → fixed
Updated•11 years ago
|
Attachment #8395275 -
Flags: approval-mozilla-beta?
Attachment #8395275 -
Flags: approval-mozilla-beta+
Attachment #8395275 -
Flags: approval-mozilla-aurora?
Attachment #8395275 -
Flags: approval-mozilla-aurora+
Updated•11 years ago
|
Keywords: checkin-needed
Comment 16•11 years ago
|
||
Keywords: checkin-needed
Comment 17•11 years ago
|
||
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).
Status: RESOLVED → VERIFIED
Comment 18•11 years ago
|
||
\o/, belatedly. Thanks for the patch!
You need to log in
before you can comment on or make changes to this bug.
Description
•