1179479 - [tablet] Tint private browsing toolbar buttons

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Follow-up to bug 864958.

via bug 864958 comment 48:

This is looking great!! But we'll need to tint those icons I think.

Namely, Back (active), forward, refresh, tabs and menu overflow.

Can we tint them to be Tabs tray icon grey? (#AFB1B3). As for the visual feedback, we can use Text and tabs tray grey (#363B40).

In the special case of the "inactive back button", we can use Toolbar icon grey (#5F6368)

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Anthony, I assume this has to land with bug 864958 in 41?

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

This only affects tablet.

Comment 3

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Michael Comella (:mcomella) from comment #1)
> Anthony, I assume this has to land with bug 864958 in 41?

If possible, yes please!

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: WIP w/ extra stuff I tried

Just want to put this here.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Screenshot of pb magnifying glass

Note to self to change the magnifying glass color too.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Anthony, the color of the toolbar text in private browsing mode is #DDDDDD, which is not in the palette - what should it be?

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

So I don't need to find where to change the color in comment 6 again:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/color/url_bar_title.xml

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

This first patch is intentionally simple and incomplete. Still TODO:
* Bookmarks icon (on xlarge tablets)
* Magnifying glass in the browser toolbar
* Hit areas

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Martyn, I can't figure out why the bookmarks icon from the patch in comment 8 doesn't change color - I apply a color filter and it remains unchanged, unlike the reload button. My best guess was that we set the icon frequently but I commented out all the places I think we do that and it still does not work.

Any ideas?

Comment 10

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Screenshot of disabled back button

(In reply to Michael Comella (:mcomella) from comment #0)
> In the special case of the "inactive back button", we can use Toolbar icon
> grey (#5F6368)

Anthony, what color should the back button be in normal browsing, disabled? We're currently using alpha values (61 in a range of 0-255) to determine the back button color.

Note that this may end up being non-trivial.

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

In the upcoming patch, I set the hit area to the spec in comment 0, but swapped the focused value (which was the same as the spec) with the hit area's previous value of @color/placeholder_active_grey.

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8638194 [details]
MozReview Request: Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

This first patch is intentionally simple and incomplete. Still TODO:
* Bookmarks icon (on xlarge tablets)
* Magnifying glass in the browser toolbar
* Hit areas

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1179479 - Set tablet private browsing hit area to spec. r=mhaigh

Bug 1179479 - Set tablet private browsing hit area to spec. r=mhaigh

Comment 14

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Michael Comella (:mcomella) from comment #6)
> Anthony, the color of the toolbar text in private browsing mode is #DDDDDD,
> which is not in the palette - what should it be?

It should follow the same spec as the phone's UI. Which would be Placeholder grey (#777777) and white (for active).

(In reply to Michael Comella (:mcomella) from comment #10)
> Created attachment 8638233 [details]
> Screenshot of disabled back button
> 
> (In reply to Michael Comella (:mcomella) from comment #0)
> > In the special case of the "inactive back button", we can use Toolbar icon
> > grey (#5F6368)
> 
> Anthony, what color should the back button be in normal browsing, disabled?
> We're currently using alpha values (61 in a range of 0-255) to determine the
> back button color.
> 
> Note that this may end up being non-trivial.

If we set it at 61 right now for normal, can I see what it looks like if we keep it at 61 alpha but for the current private browsing icon colors (tabs tray icon grey - #AFB1B3)

Comment 15

[Martyn Haigh (:mhaigh)]

Comment on attachment 8638238 [details]
MozReview Request: Bug 1179479 - Set tablet private browsing hit area to spec. r=mhaigh

https://reviewboard.mozilla.org/r/14079/#review13057

Ship It!

Comment 16

[Martyn Haigh (:mhaigh)]

Comment on attachment 8638194 [details]
MozReview Request: Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

https://reviewboard.mozilla.org/r/14041/#review13059

::: mobile/android/base/resources/layout-large-v11/new_tablet_tabs_counter.xml:7
(Diff revision 2)
> +                                         android:id="@+id/tabs_counter_text_view"

Do you use this ID anywhere, or is this part of the upcoming work still left to do?

Comment 17

[Martyn Haigh (:mhaigh)]

Comment on attachment 8638194 [details]
MozReview Request: Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

https://reviewboard.mozilla.org/r/14041/#review13061

Ship It!

Comment 18

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://reviewboard.mozilla.org/r/14041/#review13105

::: mobile/android/base/resources/layout-large-v11/new_tablet_tabs_counter.xml:7
(Diff revision 2)
> +                                         android:id="@+id/tabs_counter_text_view"

Unused - I think this was part of my WIP.

Comment 19

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

bug 1189585 adds a useful DrawableUtil.tintDrawable method that will be used for the magnifying glass.

Comment 20

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8638194 [details]
MozReview Request: Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

This first patch is intentionally simple and incomplete. Still TODO:
* Bookmarks icon (on xlarge tablets)
* Magnifying glass in the browser toolbar
* Hit areas

Comment 21

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8638238 [details]
MozReview Request: Bug 1179479 - Set tablet private browsing hit area to spec. r=mhaigh

Bug 1179479 - Set tablet private browsing hit area to spec. r=mhaigh

Comment 22

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1179479 - Correct color of url bar text in private mode. r=mhaigh

Bug 1179479 - Correct color of url bar text in private mode. r=mhaigh

Comment 23

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1179479 - Set search icon to tabs icon grey when in private mode. r=mhaigh

Bug 1179479 - Set search icon to white when in private mode. r=mhaigh

Comment 24

[Martyn Haigh (:mhaigh)]

Comment on attachment 8641436 [details]
MozReview Request: Bug 1179479 - Correct color of url bar text in private mode. r=mhaigh

https://reviewboard.mozilla.org/r/14525/#review13153

Ship It!

Comment 25

[Martyn Haigh (:mhaigh)]

Comment on attachment 8641437 [details]
MozReview Request: Bug 1179479 - Set search icon to tabs icon grey when in private mode. r=mhaigh

https://reviewboard.mozilla.org/r/14527/#review13157

Ship It!

::: mobile/android/base/toolbar/ToolbarEditText.java:170
(Diff revision 1)
> -            setCompoundDrawablesWithIntrinsicBounds(R.drawable.search_icon_inactive, 0, 0, 0);
> +            if (isPrivateMode()) {

Can we remove R.drawable.search_icon_inactive now?

Comment 26

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://reviewboard.mozilla.org/r/14527/#review13157

> Can we remove R.drawable.search_icon_inactive now?

It's still used:

https://mxr.mozilla.org/mozilla-central/search?string=search_icon_inactive&find=mobile%2Fandroid&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

bug 1167446 is meant to remove it.

Comment 27

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Anthony Lam (:antlam) from comment #14)
> > Anthony, what color should the back button be in normal browsing, disabled?
> > We're currently using alpha values (61 in a range of 0-255) to determine the
> > back button color.
> 
> If we set it at 61 right now for normal, can I see what it looks like if we
> keep it at 61 alpha but for the current private browsing icon colors (tabs
> tray icon grey - #AFB1B3)

What do you mean here? Do you mean the screenshot in comment 10? That's 61 alpha in pb icon colors.

Comment 28

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Yep!

Comment 29

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: White magnifying glass

The magnifying glass is pretty bright as #FFF, Anthony - what do you think?

Comment 30

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

It does look a bit bright. But it was intended to draw attention to that since the users' keyboard is up. We could also use the "tabs tray icon grey" (#AFB1B3) just to dial it back a bit :)

Comment 31

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Comment on attachment 8641961 [details]
White magnifying glass

This does look brighter in practice. Sure, let's try our tabs tray icon grey instead.

Comment 32

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Tabs tray icon grey magnifying glass

(In reply to Anthony Lam (:antlam) from comment #31)
> This does look brighter in practice. Sure, let's try our tabs tray icon grey
> instead.

I'm guessing you just wanted the magnifying glass changed? The text looks rather dull now.

Comment 33

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Tabs tray icon grey mag. glass w/ white font

I think this looks good.

Comment 34

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Michael Comella (:mcomella) from comment #9)
> Martyn, I can't figure out why the bookmarks icon from the patch in comment
> 8 doesn't change color - I apply a color filter and it remains unchanged,
> unlike the reload button.

MenuItemActionBar.setEnabled sets a color filter to change the color of a disabled item, overriding the values we provide. Working on a solution...

Comment 35

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8638194 [details]
MozReview Request: Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

This first patch is intentionally simple and incomplete. Still TODO:
* Bookmarks icon (on xlarge tablets)
* Magnifying glass in the browser toolbar
* Hit areas

Comment 36

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8638238 [details]
MozReview Request: Bug 1179479 - Set tablet private browsing hit area to spec. r=mhaigh

Bug 1179479 - Set tablet private browsing hit area to spec. r=mhaigh

Comment 37

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8641436 [details]
MozReview Request: Bug 1179479 - Correct color of url bar text in private mode. r=mhaigh

Bug 1179479 - Correct color of url bar text in private mode. r=mhaigh

Comment 38

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8641437 [details]
MozReview Request: Bug 1179479 - Set search icon to tabs icon grey when in private mode. r=mhaigh

Bug 1179479 - Set search icon to tabs icon grey when in private mode. r=mhaigh

Comment 39

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1179479 - Invalidate Themed* after refreshing drawable state. r=mhaigh

Bug 1179479 - Tint bookmark button on tablet in private browsing mode. r=mhaigh

This fixed a bug where we could not set color filters on
MenuItemActionBars because isEnabled used color filters to set the
appropriate color, overwriting whatever was previously set. I
switched it to use alpha, like back/forward (which have UX approval
for their alpha colors), instead of managing the enabled/disabled
state and color filters myself. Reload wasn't broken in the earlier
patch because isEnabled was not called on it.

Comment 40

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8642695 [details]
MozReview Request: Bug 1179479 - Invalidate Themed* after refreshing drawable state. r=mhaigh

This changes the color of the menu items in the 3-dot menu: nevermind.

Comment 41

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Comment on attachment 8642663 [details]
Tabs tray icon grey magnifying glass

I like the white one better too

Comment 42

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Comment on attachment 8642666 [details]
Tabs tray icon grey mag. glass w/ white font

Plus!

Comment 43

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8638194 [details]
MozReview Request: Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

This first patch is intentionally simple and incomplete. Still TODO:
* Bookmarks icon (on xlarge tablets)
* Magnifying glass in the browser toolbar
* Hit areas

Comment 44

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8638238 [details]
MozReview Request: Bug 1179479 - Set tablet private browsing hit area to spec. r=mhaigh

Bug 1179479 - Set tablet private browsing hit area to spec. r=mhaigh

Comment 45

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8641436 [details]
MozReview Request: Bug 1179479 - Correct color of url bar text in private mode. r=mhaigh

Bug 1179479 - Correct color of url bar text in private mode. r=mhaigh

Comment 46

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8641437 [details]
MozReview Request: Bug 1179479 - Set search icon to tabs icon grey when in private mode. r=mhaigh

Bug 1179479 - Set search icon to tabs icon grey when in private mode. r=mhaigh

Comment 47

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8642695 [details]
MozReview Request: Bug 1179479 - Invalidate Themed* after refreshing drawable state. r=mhaigh

Bug 1179479 - Invalidate Themed* after refreshing drawable state. r=mhaigh

afaict, refreshDrawableState only sets the appropriate back-end bits to change
the state but does not request a redraw. For example, refreshDrawableState
eventually calls Drawable.setState, whose docs specify:

  If the new state you are supplying causes the appearance of the
  Drawable to change, then it is responsible for calling invalidateSelf
  in order to have itself redrawn, and true will be returned from
  this function.

Notably, there are no other calls to invalidate in the call hierarchy so I
added one.

The reason this (sometimes) worked before is because the views would be
invalidated in some other way, e.g. setDrawable would be called.

Without this patch, the next patch would update the reload menu icon on press
(i.e. another invalidate call) but not when the tab was switched from
non-private to private.

Comment 48

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1179479 - Tint menuItemActionBar buttons via tint list. r=mhaigh

Bug 1179479 - Tint menuItemActionBar buttons via tint list. r=mhaigh

Note that this undoes a small change I made previously in this series to set
the color of the action bar icons via setColorFilter.

Comment 49

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1179479 - Set the 3-dot menu MenuItemActionBar colors. r=mhaigh

Bug 1179479 - Set the 3-dot menu MenuItemActionBar colors. r=mhaigh

The tint for disabled icons broke when I made setEnabled no longer use a
colorFilter to change the color of a disabled view.

Comment 50

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(42 w/ tracking protection)

Comment 51

[Martyn Haigh (:mhaigh)]

Comment on attachment 8644011 [details]
MozReview Request: Bug 1179479 - Set the 3-dot menu MenuItemActionBar colors. r=mhaigh

https://reviewboard.mozilla.org/r/15195/#review13913

Ship It!

Comment 52

[Martyn Haigh (:mhaigh)]

Comment on attachment 8642695 [details]
MozReview Request: Bug 1179479 - Invalidate Themed* after refreshing drawable state. r=mhaigh

https://reviewboard.mozilla.org/r/14769/#review13915

There's so much duplicated code in these files, it makes my eyes hurt!  It'd be nice to find a way of wrapping this functionality over the base classes.

Comment 53

[Martyn Haigh (:mhaigh)]

Comment on attachment 8644010 [details]
MozReview Request: Bug 1179479 - Tint menuItemActionBar buttons via tint list. r=mhaigh

https://reviewboard.mozilla.org/r/15193/#review13917

Ship It!

::: mobile/android/base/menu/MenuItemActionBar.java:8
(Diff revision 1)
> +import android.content.res.TypedArray;

Move these imports below the moz imports

::: mobile/android/base/util/DrawableUtil.java:29
(Diff revision 1)
> +        final Drawable wrappedDrawable = DrawableCompat.wrap(drawable.mutate()).mutate();

Why the double mutate call here?

Comment 54

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://reviewboard.mozilla.org/r/14769/#review13915

To be fair, `Themed*` are auto-generated so looking at the ThemedView.java.frag should be sufficient.

That being said, I'm not sure there's a better way to do this - we can have each class call out to a class instance (as a member variable) that encapsulates the functionality, removing duplication. However, we might run into `protected` access issues (i.e. the only reason we can call these methods is because we're extending the View classes). Additionally, since we have to extend these View base classes, the auto-generated templates would still need to exist to call out to this class instance.

Comment 55

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://reviewboard.mozilla.org/r/15193/#review13941

::: mobile/android/base/util/DrawableUtil.java:29
(Diff revision 1)
> +        final Drawable wrappedDrawable = DrawableCompat.wrap(drawable.mutate()).mutate();

Unintentional.

Comment 56

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

url:        https://hg.mozilla.org/integration/fx-team/rev/6e0fbdd885aa887b0b0f68c10421b252afa94688
changeset:  6e0fbdd885aa887b0b0f68c10421b252afa94688
user:       Michael Comella <[e-mail]>
date:       Thu Jul 23 15:18:23 2015 -0700
description:
Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

This first patch is intentionally simple and incomplete. Still TODO:
* Bookmarks icon (on xlarge tablets)
* Magnifying glass in the browser toolbar
* Hit areas

url:        https://hg.mozilla.org/integration/fx-team/rev/e0ef8ee4f9d69d9941678e4c834af2168224b27e
changeset:  e0ef8ee4f9d69d9941678e4c834af2168224b27e
user:       Michael Comella <[e-mail]>
date:       Thu Jul 23 17:13:35 2015 -0700
description:
Bug 1179479 - Set tablet private browsing hit area to spec. r=mhaigh

url:        https://hg.mozilla.org/integration/fx-team/rev/a1ffe0d6a9ee26fde54adb455d61b7ce041c8e4a
changeset:  a1ffe0d6a9ee26fde54adb455d61b7ce041c8e4a
user:       Michael Comella <[e-mail]>
date:       Thu Jul 30 19:07:00 2015 -0700
description:
Bug 1179479 - Correct color of url bar text in private mode. r=mhaigh

url:        https://hg.mozilla.org/integration/fx-team/rev/b163bf0c487ff9f219b1bf9256bcc13dda7c85e9
changeset:  b163bf0c487ff9f219b1bf9256bcc13dda7c85e9
user:       Michael Comella <[e-mail]>
date:       Thu Jul 30 19:18:29 2015 -0700
description:
Bug 1179479 - Set search icon to tabs icon grey when in private mode. r=mhaigh

url:        https://hg.mozilla.org/integration/fx-team/rev/ecb79b9ae0f9a4e1b8605bca88a313760a9b89f5
changeset:  ecb79b9ae0f9a4e1b8605bca88a313760a9b89f5
user:       Michael Comella <[e-mail]>
date:       Wed Aug 05 14:17:49 2015 -0700
description:
Bug 1179479 - Invalidate Themed* after refreshing drawable state. r=mhaigh

afaict, refreshDrawableState only sets the appropriate back-end bits to change
the state but does not request a redraw. For example, refreshDrawableState
eventually calls Drawable.setState, whose docs specify:

  If the new state you are supplying causes the appearance of the
  Drawable to change, then it is responsible for calling invalidateSelf
  in order to have itself redrawn, and true will be returned from
  this function.

Notably, there are no other calls to invalidate in the call hierarchy so I
added one.

The reason this (sometimes) worked before is because the views would be
invalidated in some other way, e.g. setDrawable would be called.

Without this patch, the next patch would update the reload menu icon on press
(i.e. another invalidate call) but not when the tab was switched from
non-private to private.

url:        https://hg.mozilla.org/integration/fx-team/rev/52f19dfa18a5c79599b417291a843095ce8fc225
changeset:  52f19dfa18a5c79599b417291a843095ce8fc225
user:       Michael Comella <[e-mail]>
date:       Wed Aug 05 15:13:38 2015 -0700
description:
Bug 1179479 - Tint menuItemActionBar buttons via tint list. r=mhaigh

Note that this undoes a small change I made previously in this series to set
the color of the action bar icons via setColorFilter.

url:        https://hg.mozilla.org/integration/fx-team/rev/4b9d1ec3213d125b886ae708467eb8d040cb9e1c
changeset:  4b9d1ec3213d125b886ae708467eb8d040cb9e1c
user:       Michael Comella <[e-mail]>
date:       Wed Aug 05 15:38:57 2015 -0700
description:
Bug 1179479 - Set the 3-dot menu MenuItemActionBar colors. r=mhaigh

The tint for disabled icons broke when I made setEnabled no longer use a
colorFilter to change the color of a disabled view.

Comment 57

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8638194 [details]
MozReview Request: Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh


This request applies to all changesets that were pushed.

Approval Request Comment
[Feature/regressing bug #]: Tracking protection initiative in 42 and bug 864958
[User impact if declined]:
  Users on tablet will have a toolbar with hard-to-read, low contrast icons since they're tinted the wrong colors.

[Describe test coverage new/current, TreeHerder]: Tested locally
[Risks and why]: The changes are small but there are a lot of them so maybe I missed something, particularly on edge cases or Android fragmentation. However, the changes are all color related so the consequences are low (e.g. screwed up colors).

[String/UUID change made/needed]: None

Comment 58

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6e0fbdd885aa
https://hg.mozilla.org/mozilla-central/rev/e0ef8ee4f9d6
https://hg.mozilla.org/mozilla-central/rev/a1ffe0d6a9ee
https://hg.mozilla.org/mozilla-central/rev/b163bf0c487f
https://hg.mozilla.org/mozilla-central/rev/ecb79b9ae0f9
https://hg.mozilla.org/mozilla-central/rev/52f19dfa18a5
https://hg.mozilla.org/mozilla-central/rev/4b9d1ec3213d

Comment 59

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8638194 [details]
MozReview Request: Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

Cancel uplift approval for potential regression in bug 1193950.

Comment 60

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8638194 [details]
MozReview Request: Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

Approval request in comment 57. This should be uplifted with bug 1193950.

Comment 61

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8638194 [details]
MozReview Request: Bug 1179479 - Tint color of easy-to-do tablet toolbar icons. r=mhaigh

New feature, let's take it.

Comment 62

[Sylvestre Ledru [:Sylvestre]]

Michael, next time, please be explicit on the uplift. I think I am correct but I could have missed one (or took a wrong patch). Thanks! :)

Comment 63

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #62)
> Michael, next time, please be explicit on the uplift. I think I am correct
> but I could have missed one (or took a wrong patch). Thanks! :)

I don't usually push the latest versions of these patches (after review) back to reviewboard, and just land the updated versions in the tree, so in comment 57 I mentioned: "This request applies to all changesets that were pushed."

Would it be better to push the updated commits and request uplift on them individually?

Comment 64

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Also, bug 1194659 should be uplifted with this too.

Comment 65

[Sylvestre Ledru [:Sylvestre]]

Michael, I don't know what is best for the sheriff (but I am pretty sure that they prefer when it is explicit), I just prefer to have a clean state in bugzilla :)

Comment 66

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #65)
> Michael, I don't know what is best for the sheriff (but I am pretty sure
> that they prefer when it is explicit), I just prefer to have a clean state
> in bugzilla :)

Okay – it's a pain to push the latest versions of these commits to reviewboard again but I'll do so in the future.

@Sheriffs: Please land the commits landed in comment 56.

Comment 67

[Ryan VanderMeulen [:RyanVM]]

We typically transplant the patches as-landed on m-c unless there's a branch patch attached, FWIW.

Comment 68

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d393cac4e944
https://hg.mozilla.org/releases/mozilla-aurora/rev/677cba8fd20b
https://hg.mozilla.org/releases/mozilla-aurora/rev/324ad05911d1
https://hg.mozilla.org/releases/mozilla-aurora/rev/ae70f5d5f5b7
https://hg.mozilla.org/releases/mozilla-aurora/rev/9032008d5625
https://hg.mozilla.org/releases/mozilla-aurora/rev/7c092142102f
https://hg.mozilla.org/releases/mozilla-aurora/rev/33c0b0175e5f