Closed
Bug 995300
Opened 10 years ago
Closed 10 years ago
When the location bar grows vertically due to an increased font size, the forward button should grow as well to match the location bar
Categories
(Firefox :: Theme, defect)
Firefox
Theme
Tracking
()
RESOLVED
FIXED
Firefox 32
People
(Reporter: dao, Assigned: dao)
References
Details
Attachments
(1 file)
2.04 KB,
patch
|
mikedeboer
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Screenshot: http://i.imgur.com/XGuOF8i.png This probably affects all platforms but doesn't matter on OS X where users presumably can't change the system font size.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → dao
Assignee | ||
Comment 2•10 years ago
|
||
Quick and dirty. This stretches the icon, which isn't ideal but better than the current situation where the borders don't connect. Fixing this without stretching the icon would involve moving the button styling from the .toolbarbutton-icon to #forward-button.
Attachment #8416461 -
Flags: review?(mdeboer)
Comment 3•10 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #2) > Fixing this without stretching the icon would involve moving the button > styling from the .toolbarbutton-icon to #forward-button. What happens when you give the #forward-button > .toolbarbutton-icon a fixed height?
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Mike de Boer [:mikedeboer] from comment #3) > (In reply to Dão Gottwald [:dao] from comment #2) > > Fixing this without stretching the icon would involve moving the button > > styling from the .toolbarbutton-icon to #forward-button. > > What happens when you give the #forward-button > .toolbarbutton-icon a fixed > height? It has an implied fixed height right now, causing this bug.
Updated•10 years ago
|
Status: NEW → ASSIGNED
Comment 5•10 years ago
|
||
Comment on attachment 8416461 [details] [diff] [review] patch Review of attachment 8416461 [details] [diff] [review]: ----------------------------------------------------------------- We'd need to move the styling to the #forward-button indeed. Even though the stretched icon is visually jarring to me, I still feel it's an improvement, like you said. Can you file a follow-up for that? ::: browser/themes/linux/browser.css @@ +739,5 @@ > transform: scaleX(-1); > } > > #forward-button { > + -moz-box-align: stretch; /* let the button shape grow vertically with the location bar */ nit: can you put the comment above this line? ::: browser/themes/windows/browser.css @@ +876,5 @@ > > /* unified back/forward button */ > > #forward-button { > + -moz-box-align: stretch; /* let the button shape grow vertically with the location bar */ nit: same here.
Attachment #8416461 -
Flags: review?(mdeboer) → review+
Assignee | ||
Comment 6•10 years ago
|
||
> > #forward-button { > > + -moz-box-align: stretch; /* let the button shape grow vertically with the location bar */ > > nit: can you put the comment above this line? Oops, I imported the patch and meant to make the above change, but then I a got distracted and ended up pushing it unmodified: https://hg.mozilla.org/integration/fx-team/rev/5e1f22e16fa4 I did originally put that comment on the same line on purpose, to make it clearer that it's only for that property rather than the entire rule...
Comment 7•10 years ago
|
||
*slap* I'll let it slide, only this one time... :-P
Comment 8•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5e1f22e16fa4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 8416461 [details] [diff] [review] patch [Approval Request Comment] Bug caused by (feature/regressing bug #): not sure... User impact if declined: the forward button may have a different height than the location bar (http://i.imgur.com/XGuOF8i.png) Testing completed (on m-c, etc.): on m-c Risk to taking this patch (and alternatives if risky): low String or IDL/UUID changes made by this patch: none
Attachment #8416461 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
status-firefox31:
--- → affected
status-firefox32:
--- → fixed
Updated•10 years ago
|
Attachment #8416461 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Updated•10 years ago
|
Flags: in-testsuite? → in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•