[tablet] Tint private browsing toolbar buttons

RESOLVED FIXED in Firefox 42

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mcomella, Assigned: mcomella)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 43
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed, firefox43 fixed, fennec42+)

Details

Attachments

(12 attachments, 1 obsolete attachment)

88.46 KB, image/png
Details
40 bytes, text/x-review-board-request
mhaigh
: review+
Details
66.44 KB, image/png
Details
40 bytes, text/x-review-board-request
mhaigh
: review+
Details
40 bytes, text/x-review-board-request
mhaigh
: review+
Details
40 bytes, text/x-review-board-request
mhaigh
: review+
Details
333.32 KB, image/png
antlam
: feedback-
Details
90.59 KB, image/png
antlam
: feedback-
Details
90.89 KB, image/png
antlam
: feedback+
Details
40 bytes, text/x-review-board-request
mhaigh
: review+
Details
40 bytes, text/x-review-board-request
mhaigh
: review+
Details
40 bytes, text/x-review-board-request
mhaigh
: review+
Details
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)
Anthony, I assume this has to land with bug 864958 in 41?
Flags: needinfo?(alam)
This only affects tablet.
Summary: Tint private browsing toolbar buttons → [tablet] Tint private browsing toolbar buttons
(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!
Flags: needinfo?(alam)
Assignee: nobody → michael.l.comella
Created attachment 8637622 [details] [diff] [review]
WIP w/ extra stuff I tried

Just want to put this here.
Created attachment 8638160 [details]
Screenshot of pb magnifying glass

Note to self to change the magnifying glass color too.
Anthony, the color of the toolbar text in private browsing mode is #DDDDDD, which is not in the palette - what should it be?
Flags: needinfo?(alam)
Created 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
Attachment #8637622 - Attachment is obsolete: true
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?
Flags: needinfo?(mhaigh)
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.
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 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
Attachment #8638194 - Flags: review?(mhaigh)
Created 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
Attachment #8638238 - Flags: review?(mhaigh)
(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)
Flags: needinfo?(alam) → needinfo?(michael.l.comella)
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!
Attachment #8638238 - Flags: review?(mhaigh) → review+
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?
Attachment #8638194 - Flags: review?(mhaigh)

Updated

3 years ago
Attachment #8638194 - Flags: review+
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!
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.
bug 1189585 adds a useful DrawableUtil.tintDrawable method that will be used for the magnifying glass.
Blocks: 1189585
Flags: needinfo?(michael.l.comella)
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 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
Created 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
Attachment #8641436 - Flags: review?(mhaigh)
Created 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 white when in private mode. r=mhaigh
Attachment #8641437 - Flags: review?(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!
Attachment #8641436 - Flags: review?(mhaigh) → review+
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?
Attachment #8641437 - Flags: review?(mhaigh) → review+
(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.
Flags: needinfo?(alam)
Yep!
Flags: needinfo?(alam)
Created attachment 8641961 [details]
White magnifying glass

The magnifying glass is pretty bright as #FFF, Anthony - what do you think?
Attachment #8641961 - Flags: feedback?(alam)
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 on attachment 8641961 [details]
White magnifying glass

This does look brighter in practice. Sure, let's try our tabs tray icon grey instead.
Attachment #8641961 - Flags: feedback?(alam) → feedback-
Created attachment 8642663 [details]
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.
Attachment #8642663 - Flags: feedback?(alam)
Created attachment 8642666 [details]
Tabs tray icon grey mag. glass w/ white font

I think this looks good.
Attachment #8642666 - Flags: feedback?(alam)
(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...
Flags: needinfo?(mhaigh)
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 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 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 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
Attachment #8641437 - Attachment description: MozReview Request: Bug 1179479 - Set search icon to white when in private mode. r=mhaigh → MozReview Request: Bug 1179479 - Set search icon to tabs icon grey when in private mode. r=mhaigh
Created attachment 8642695 [details]
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.
Attachment #8642695 - Flags: review?(mhaigh)
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.
Attachment #8642695 - Flags: review?(mhaigh)
Comment on attachment 8642663 [details]
Tabs tray icon grey magnifying glass

I like the white one better too
Attachment #8642663 - Flags: feedback?(alam) → feedback-
Comment on attachment 8642666 [details]
Tabs tray icon grey mag. glass w/ white font

Plus!
Attachment #8642666 - Flags: feedback?(alam) → feedback+
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 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 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 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 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.
Attachment #8642695 - Attachment description: MozReview Request: Bug 1179479 - Tint bookmark button on tablet in private browsing mode. r=mhaigh → MozReview Request: Bug 1179479 - Invalidate Themed* after refreshing drawable state. r=mhaigh
Attachment #8642695 - Flags: review?(mhaigh)
Created attachment 8644010 [details]
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.
Attachment #8644010 - Flags: review?(mhaigh)
Created attachment 8644011 [details]
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.
Attachment #8644011 - Flags: review?(mhaigh)
(42 w/ tracking protection)
tracking-fennec: --- → 42+
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!
Attachment #8644011 - Flags: review?(mhaigh) → review+
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.
Attachment #8642695 - Flags: review?(mhaigh) → review+
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?
Attachment #8644010 - Flags: review?(mhaigh) → review+
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.
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.
url:        https://hg.mozilla.org/integration/fx-team/rev/6e0fbdd885aa887b0b0f68c10421b252afa94688
changeset:  6e0fbdd885aa887b0b0f68c10421b252afa94688
user:       Michael Comella <michael.l.comella@gmail.com>
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 <michael.l.comella@gmail.com>
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 <michael.l.comella@gmail.com>
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 <michael.l.comella@gmail.com>
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 <michael.l.comella@gmail.com>
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 <michael.l.comella@gmail.com>
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 <michael.l.comella@gmail.com>
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 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
Attachment #8638194 - Flags: approval-mozilla-aurora?
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.
Attachment #8638194 - Flags: approval-mozilla-aurora?
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.
Attachment #8638194 - Flags: approval-mozilla-aurora?
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.
Attachment #8638194 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8638238 - Flags: approval-mozilla-aurora+
Attachment #8641436 - Flags: approval-mozilla-aurora+
Attachment #8641437 - Flags: approval-mozilla-aurora+
Attachment #8642695 - Flags: approval-mozilla-aurora+
Attachment #8644010 - Flags: approval-mozilla-aurora+
Attachment #8644011 - Flags: approval-mozilla-aurora+
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! :)
(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?
Flags: needinfo?(sledru)
Also, bug 1194659 should be uplifted with this too.
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 :)
Flags: needinfo?(sledru)
(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.
We typically transplant the patches as-landed on m-c unless there's a branch patch attached, FWIW.
You need to log in before you can comment on or make changes to this bug.