1254944 - Move changes to generated ThemedImageButton to generator script

Status: RESOLVED FIXED

(Firefox for Android :: General, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

We generate the Themed*.java source files by running generate_themed_views.py.

However in bug 1227120 ThemedImageButton.java has been modified. This change will be lost the next time we generate those source files from ThemedView.java.frag. (Just saw this happening in bug 1193208).

Change:
https://hg.mozilla.org/mozilla-central/diff/23041118e319/mobile/android/base/java/org/mozilla/gecko/widget/themed/ThemedImageButton.java

Comment 1

[Chenxia Liu [:liuche]]

Whoops, I missed that comment at the top of the file. I'll take care of fixing this.

Comment 2

[Chenxia Liu [:liuche]]

Attachment: MozReview Request: Bug 1254944 - Move changes to generated ThemedImageButton to generator script. r=grisha

Review commit: https://reviewboard.mozilla.org/r/38965/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38965/

Comment 3

[Chenxia Liu [:liuche]]

Nested #ifdefs work fine, and I also verified that a build with these generated changes works as expected.

Comment 4

[Chenxia Liu [:liuche]]

Comment on attachment 8728534 [details]
MozReview Request: Bug 1254944 - Move changes to generated ThemedImageButton to generator script. r=grisha

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38965/diff/1-2/

Comment 5

[Chenxia Liu [:liuche]]

This is another small review, and feel free to send it back to mcomella or sebastian. Again, some reviewboard exposure and taking a peek at how some of our button code is.

See the changeset mentioned in comment #1 for the code these changes should generate.

Comment 6

[:Grisha Kruglov]

Comment on attachment 8728534 [details]
MozReview Request: Bug 1254944 - Move changes to generated ThemedImageButton to generator script. r=grisha

https://reviewboard.mozilla.org/r/38965/#review35977

Looks good as far as I can tell, .frag changes correspond to what changed in the .java file; I'm lacking context as to what's required to fully make a change like this, but the diff looks entirely reasonable.

::: mobile/android/base/java/org/mozilla/gecko/widget/themed/generate_themed_views.py:15
(Diff revision 2)
> -(ThemedView.java.frag) and run the script.  Use version control to
> +(ThemedView.java.frag) and run the script using 'mach python <script.py>'.  Use version control to

yay better docs!

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/5d690a383e54

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5d690a383e54

Comment 9

[Chenxia Liu [:liuche]]

Comment on attachment 8728534 [details]
MozReview Request: Bug 1254944 - Move changes to generated ThemedImageButton to generator script. r=grisha

Approval Request Comment
[Feature/regressing bug #]: bug 1227120 - update color of filled bookmark star, made changes to generated output file, not source
[User impact if declined]: Changes will be overwritten
[Describe test coverage new/current, TreeHerder]: local
[Risks and why]: low, tested code generation, and verified output is the same as the original changes
[String/UUID change made/needed]: none

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8728534 [details]
MozReview Request: Bug 1254944 - Move changes to generated ThemedImageButton to generator script. r=grisha

Seems low risk UI change, Aurora47+

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e58f03852f8f