Closed Bug 1114570 Opened 7 years ago Closed 7 years ago

[devedition] Wrong searchbar go button icon

Categories

(Firefox :: Theme, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 37

People

(Reporter: ntim, Assigned: ntim)

References

Details

(Whiteboard: [devedition-polish])

Attachments

(1 file)

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])
Whiteboard: [devedition-polish]
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)
Brian, want to have a look? I promise fast reviews! :-)
Flags: needinfo?(ntim007) → needinfo?(bgrinstead)
Assignee: nobody → ntim007
Status: NEW → ASSIGNED
(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.
Attached patch PatchSplinter Review
Flags: needinfo?(bgrinstead)
Attachment #8540303 - Flags: review?(gijskruitbosch+bugs)
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-
(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).
Should we wait for bug 1095836 or can we land this fix ?
Flags: needinfo?(gijskruitbosch+bugs)
(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)
Duplicate of this bug: 1112647
Duplicate of this bug: 1118807
Tim, could you update this patch as noted in Comment 10?
Flags: needinfo?(ntim007)
(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 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+
Flags: needinfo?(gijskruitbosch+bugs)
Keywords: checkin-needed
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/7a776d0c87e6
Whiteboard: [devedition-polish] → [fixed-in-fx-team][devedition-polish]
https://hg.mozilla.org/mozilla-central/rev/7a776d0c87e6
Status: ASSIGNED → RESOLVED
Closed: 7 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.