1199041 - Improve comment in DrawableUtil.tintDrawableWithStateList

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

Chenxia pointed out this was confusing. Currently:

34      * Tints the given drawable with the given tint list and returns it. Note that this
35      * transformation does not occur in place on pre-Lollipop devices (bug 1193950).

"in place" is inaccurate – it really means "mutation" but doesn't really get the point across.

How about:

"Tints the given drawable with the given tint list and returns it. Use the returned Drawable, rather than the argument, because the the argument Drawable is not in a consistent state across Android versions, due to the Support Library implementation across versions."

Chenxia, thoughts?

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/util/DrawableUtil.java#34

Comment 1

[Chenxia Liu [:liuche]]

This is better, but could be better.

How about:

"Tints the given drawable with the given tint list and returns it. 'wrap()' is called on the Drawable so that it can be consumed by all API versions because the Drawable implementation is not consistent across versions."

Comment 2

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

Attachment: MozReview Request: Bug 1199041 - Clarify comment in DrawableUtil. r=liuche

Bug 1199041 - Clarify comment in DrawableUtil. r=liuche

Comment 3

[Chenxia Liu [:liuche]]

Comment on attachment 8655717 [details]
MozReview Request: Bug 1199041 - Clarify comment in DrawableUtil. r=liuche

https://reviewboard.mozilla.org/r/18003/#review16561

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/8df7dd2d23fd

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8df7dd2d23fd