Closed Bug 1254944 Opened 4 years ago Closed 4 years ago

Move changes to generated ThemedImageButton to generator script

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: sebastian, Assigned: liuche)

References

Details

Attachments

(1 file)

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
Depends on: 1254946
Whoops, I missed that comment at the top of the file. I'll take care of fixing this.
Assignee: nobody → liuche
Summary: Generated ThemedImageButton has been modified → Move changes to generated ThemedImageButton to generator script
Nested #ifdefs work fine, and I also verified that a build with these generated changes works as expected.
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/
Attachment #8728534 - Attachment description: MozReview Request: Bug 1254944 - Move changes to generated ThemedImageButton to generator script. r=mcomella → MozReview Request: Bug 1254944 - Move changes to generated ThemedImageButton to generator script. r=grisha
Attachment #8728534 - Flags: review?(michael.l.comella) → review?(gkruglov)
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.
Attachment #8728534 - Flags: review?(gkruglov) → review+
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!
https://hg.mozilla.org/mozilla-central/rev/5d690a383e54
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
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
Attachment #8728534 - Flags: approval-mozilla-aurora?
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+
Attachment #8728534 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.