Closed
Bug 1114570
Opened 10 years ago
Closed 9 years ago
[devedition] Wrong searchbar go button icon
Categories
(Firefox :: Theme, defect)
Firefox
Theme
Tracking
()
RESOLVED
FIXED
Firefox 37
People
(Reporter: ntim, Assigned: ntim)
References
Details
(Whiteboard: [devedition-polish])
Attachments
(1 file)
853 bytes,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
The go button should be an arrow, not a search glass. The selector used to change the icon should be specific to the non-oneoffui : searchbar:not([oneoffui])
Assignee | ||
Updated•10 years ago
|
Whiteboard: [devedition-polish]
Comment 1•10 years ago
|
||
Florian/Brian, not sure who knows about this. Tim, can you provide more details? This is working for me in beta, and right now it's not clear to me whether this is devedition (theme) only, also on Nightly, a regression, needs specific steps to reproduce, or something else.
Flags: needinfo?(ntim007)
Comment 2•10 years ago
|
||
My guess is this refers to http://hg.mozilla.org/mozilla-central/annotate/b915a50bc6be/browser/themes/shared/devedition.inc.css#l249 For the normal themes, we have 2 rules: http://hg.mozilla.org/mozilla-central/annotate/b915a50bc6be/browser/themes/linux/searchbar.css#l57 http://hg.mozilla.org/mozilla-central/annotate/b915a50bc6be/browser/themes/linux/searchbar.css#l93
Comment 3•10 years ago
|
||
Brian, want to have a look? I promise fast reviews! :-)
Flags: needinfo?(ntim007) → needinfo?(bgrinstead)
Assignee | ||
Comment 4•10 years ago
|
||
This rule [0] should also be made specific. [0] http://hg.mozilla.org/mozilla-central/annotate/b915a50bc6be/browser/themes/windows/devedition.css#l108
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → ntim007
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Tim Nguyen [:ntim] from comment #4) > This rule [0] should also be made specific. > [0] > http://hg.mozilla.org/mozilla-central/annotate/b915a50bc6be/browser/themes/ > windows/devedition.css#l108 Nope, actually, this element doesn't exist for the oneoffui search bar.
Assignee | ||
Comment 6•10 years ago
|
||
Flags: needinfo?(bgrinstead)
Attachment #8540303 -
Flags: review?(gijskruitbosch+bugs)
Comment 7•10 years ago
|
||
Comment on attachment 8540303 [details] [diff] [review] Patch Review of attachment 8540303 [details] [diff] [review]: ----------------------------------------------------------------- We should fix the arrow for the oneoffui case, too.
Attachment #8540303 -
Flags: review?(gijskruitbosch+bugs) → review-
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #7) > Comment on attachment 8540303 [details] [diff] [review] > Patch > > Review of attachment 8540303 [details] [diff] [review]: > ----------------------------------------------------------------- > > We should fix the arrow for the oneoffui case, too. The arrow used by the oneoffui is the urlbar one, and there's a bug filed about the urlbar icon being too dark : bug 1095836 (which is waiting for assets).
Assignee | ||
Comment 9•10 years ago
|
||
Should we wait for bug 1095836 or can we land this fix ?
Flags: needinfo?(gijskruitbosch+bugs)
Comment 10•10 years ago
|
||
(In reply to Tim Nguyen [:ntim] from comment #9) > Should we wait for bug 1095836 or can we land this fix ? No, and no. We should fix this to use the same icon, then when the icon gets replaced, this fix will "just work".
Flags: needinfo?(gijskruitbosch+bugs)
Comment 13•9 years ago
|
||
Tim, could you update this patch as noted in Comment 10?
Flags: needinfo?(ntim007)
Comment 14•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #10) > (In reply to Tim Nguyen [:ntim] from comment #9) > > Should we wait for bug 1095836 or can we land this fix ? > > No, and no. We should fix this to use the same icon, then when the icon gets > replaced, this fix will "just work". So, right now devedition isn't using any custom asset for that 'go' icon. We are waiting for one in Bug 1095836 - there isn't anything to switch to right now. So unless if I'm misunderstanding the request, I'd vote for landing this fix.
Flags: needinfo?(ntim007) → needinfo?(gijskruitbosch+bugs)
Comment 15•9 years ago
|
||
Comment on attachment 8540303 [details] [diff] [review] Patch Review of attachment 8540303 [details] [diff] [review]: ----------------------------------------------------------------- We can land it because it's better than nothing, but it'd be good to get an updated asset for the (default) dark case... :-\
Attachment #8540303 -
Flags: review- → review+
Updated•9 years ago
|
Flags: needinfo?(gijskruitbosch+bugs)
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 16•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/7a776d0c87e6
Whiteboard: [devedition-polish] → [fixed-in-fx-team][devedition-polish]
Comment 17•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7a776d0c87e6
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][devedition-polish] → [devedition-polish]
Target Milestone: --- → Firefox 37
You need to log in
before you can comment on or make changes to this bug.
Description
•