Expand entities at build time when generating mobile/android/base/strings.xml

RESOLVED FIXED in Firefox 47

Status

()

RESOLVED FIXED
4 years ago
3 months ago

People

(Reporter: nalexander, Assigned: nalexander)

Tracking

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

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment)

When I check a while back (3 months ago), IntelliJ did not correctly handle &entity; definitions in Android strings.xml files.  Strings with entities (in Fennec, all of them) were rendered in the IDE as blank.

One work-around is to expand the entities at build time.  It turns out this is easy; I found some Python one-liner to do this, although I can't remember what it was.  If this is cheap, we should do it to ease Android Studio integration.  I can't think of a problem, but I'd like to verify that the .ap_ file generated by aapt is identical or trivially different.

Pike, flod: can you see any problem with replacing the first strings.xml with the second would cause a problem?  This would be per-locale, at build-time, so there's no concern with dynamic expansion at run-time.

Existing fragment:

<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE resources [
<!ENTITY search_plus_content_description 'Add to search bar'>
]>
<resources>
    <string name="search_plus_content_description">&search_plus_content_description;</string>
</resources>

Proposed fragment:

<?xml version="1.0" encoding="utf-8"?>
<resources>
    <string name="search_plus_content_description">Add to search bar</string>
</resources>

Comment 1

4 years ago
I'm not sure, I don't trust Android's l10n and build system. I don't know of anything that says that that shouldn't work, but yeah, I don't know.

I'd be cautious if the replacement code wouldn't involve real XML parsers, just because it's easy to write regexps that work for 99% and then break the build.
(Assignee)

Comment 2

4 years ago
(In reply to Axel Hecht [:Pike] from comment #1)
> I'm not sure, I don't trust Android's l10n and build system. I don't know of
> anything that says that that shouldn't work, but yeah, I don't know.
> 
> I'd be cautious if the replacement code wouldn't involve real XML parsers,
> just because it's easy to write regexps that work for 99% and then break the
> build.

Roger that.  I think I used Python's lxml or ElementTree.  I certainly didn't write a parser myself, or a regex.  Thanks for fast feedback.
(Assignee)

Comment 4

3 years ago
Created attachment 8707980 [details]
MozReview Request: Bug 1074258 - Expand entities at build time when generating strings.xml. r?glandium

IntelliJ does not correctly handle &entity; definitions in Android
strings.xml files.  Strings with entities (in Fennec, all of them) are
rendered in the IDE as blank.  This patch expands the entities at
build time, improving the IntelliJ integration.

Review commit: https://reviewboard.mozilla.org/r/30941/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30941/
Attachment #8707980 - Flags: review?(mh+mozilla)
Comment on attachment 8707980 [details]
MozReview Request: Bug 1074258 - Expand entities at build time when generating strings.xml. r?glandium

https://reviewboard.mozilla.org/r/30941/#review28089

::: mobile/android/base/locales/Makefile.in:69
(Diff revision 1)
> +# name or location depend on the locale.  If it were easier to pipe
> +# between py_action invocations, we could avoid this intermediate file
> +# altogether.

It's not really possible to pipe between py_action invocations, but you can make your new action to preprocessing itself, in which case you don't need the intermediate rule.
Attachment #8707980 - Flags: review?(mh+mozilla)
(Assignee)

Comment 7

3 years ago
I was doing similar things in Gradle, so I hit this too.  I don't really care if we do this "for real" in the regular build system, since it's only for IDE users.
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Component: Build Config → Build Config & IDE Support
Product: Core → Firefox for Android

Comment 9

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f4abd2bf1fd3
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
(Assignee)

Updated

3 months ago
Blocks: 1478411
You need to log in before you can comment on or make changes to this bug.