Closed
Bug 923519
Opened 11 years ago
Closed 11 years ago
Remove resource preprocessing
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 27
People
(Reporter: bnicholson, Assigned: bnicholson)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 1 obsolete file)
4.39 KB,
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
16.86 KB,
patch
|
nalexander
:
review+
|
Details | Diff | Splinter Review |
We have a several *.xml.in resource files that are preprocessed using #filter substitution with make variables. We can factor these make variables out into strings.xml.in and reference the strings in the resource files (instead of the make vars directly) to remove the preprocessing.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #813608 -
Flags: review?(nalexander)
Comment 2•11 years ago
|
||
Comment on attachment 813608 [details] [diff] [review] Remove resource preprocessing Review of attachment 813608 [details] [diff] [review]: ----------------------------------------------------------------- lgtm. I'll test locally, and I'll land with the required android-sync github changes. ::: mobile/android/base/android-services-files.mk @@ +349,3 @@ > $(NULL) > > SYNC_PP_RES_XML := \ This should go. But it requires android-sync github changes, so I'll land with this removed. ::: mobile/android/base/locales/Makefile.in @@ +69,2 @@ > -DBRANDPATH="$(BRANDPATH)" \ > + -DMOZ_ANDROID_SHARED_ACCOUNT_TYPE=$(MOZ_ANDROID_SHARED_ACCOUNT_TYPE) \ I'm not confident that MOZ_ANDROID_SHARED_ACCOUNT_TYPE is defined here.
Attachment #813608 -
Flags: review?(nalexander) → review+
Assignee | ||
Comment 3•11 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #2) > ::: mobile/android/base/locales/Makefile.in > @@ +69,2 @@ > > -DBRANDPATH="$(BRANDPATH)" \ > > + -DMOZ_ANDROID_SHARED_ACCOUNT_TYPE=$(MOZ_ANDROID_SHARED_ACCOUNT_TYPE) \ > > I'm not confident that MOZ_ANDROID_SHARED_ACCOUNT_TYPE is defined here. I didn't get any build errors with this patch, so I think we're safe.
Comment 4•11 years ago
|
||
(In reply to Brian Nicholson (:bnicholson) from comment #3) > (In reply to Nick Alexander :nalexander from comment #2) > > ::: mobile/android/base/locales/Makefile.in > > @@ +69,2 @@ > > > -DBRANDPATH="$(BRANDPATH)" \ > > > + -DMOZ_ANDROID_SHARED_ACCOUNT_TYPE=$(MOZ_ANDROID_SHARED_ACCOUNT_TYPE) \ > > > > I'm not confident that MOZ_ANDROID_SHARED_ACCOUNT_TYPE is defined here. > > I didn't get any build errors with this patch, so I think we're safe. You won't get a compile error; make treats a missing value as an empty string, so this will compile and Sync will fail. (Check $OBJDIR/mobile/android/base/res/values/strings.xml; you'll see the shared account type is null.) So we'll need to fix this, which I don't have time to do right now.
Comment 5•11 years ago
|
||
http://tbpl.mozilla.org/?tree=Try&rev=33499b3b08b5
Comment 6•11 years ago
|
||
Comment 7•11 years ago
|
||
Comment 8•11 years ago
|
||
Comment on attachment 814142 [details] [diff] [review] Remove resource preprocessing. r=nalexander Review of attachment 814142 [details] [diff] [review]: ----------------------------------------------------------------- This is a slightly updated version of bnicholson's patch.
Attachment #814142 -
Flags: review+
Updated•11 years ago
|
Attachment #813608 -
Attachment is obsolete: true
Updated•11 years ago
|
Attachment #814141 -
Flags: review?(gps)
Assignee | ||
Comment 9•11 years ago
|
||
Comment on attachment 814142 [details] [diff] [review] Remove resource preprocessing. r=nalexander Review of attachment 814142 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/android-services-files.mk @@ +347,2 @@ > res/xml/sync_syncadapter.xml \ > res/xml/sync_options.xml \ Nit for your modifications: It looks like you wanted to reorder this alphabetically since you put sync_authenticator first, so you should move sync_options before sync_syncadapter.
Comment 10•11 years ago
|
||
Comment on attachment 814141 [details] [diff] [review] Pre: Expose MOZ_ANDROID_SHARED_{ID,ACCOUNT_TYPE} to mobile/android/**/Makefile.in. r=gps Review of attachment 814141 [details] [diff] [review]: ----------------------------------------------------------------- As much as I dislike defs.mk, r+.
Attachment #814141 -
Flags: review?(gps) → review+
Comment 11•11 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/4d996eac6432 https://hg.mozilla.org/integration/fx-team/rev/085d56ad055d
Status: NEW → ASSIGNED
Comment 12•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4d996eac6432 https://hg.mozilla.org/mozilla-central/rev/085d56ad055d
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•