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

Status: RESOLVED FIXED

(Firefox Build System :: Android Studio and Gradle Integration, defect)

Description

[Nick Alexander :nalexander [he/him]]

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

[Axel Hecht [:Pike]]

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.

Comment 2

[Nick Alexander :nalexander [he/him]]

(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.

Comment 3

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ccf4b44c518

Comment 4

[Nick Alexander :nalexander [he/him]]

Attachment: 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/

Comment 5

[Mike Hommey [:glandium]]

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.

Comment 6

[Pulsebot]

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

Comment 7

[Nick Alexander :nalexander [he/him]]

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.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/10f04b5ed971

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f4abd2bf1fd3

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/10f04b5ed971