708015 - support both xul and android UI at the same time

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P4)

Description

[Axel Hecht [:Pike]]

Seems that backchannel conversations say that we'd better be safe than sorry when it comes down to being able to build a localized xul UI.

What to do for sure, as that's just hygiene:

remove updater and crashreporter from mobile/android, neither are used, according to ted and rstrong.
remove installer for both android and xul, that's cruft from winmo


Now to the beef:

Leave mobile/android/locales/en-US/chrome and mobile/android/bases/locales/en-US alone.

Move searchplugins, profile, and chrome/overrides back to mobile/locales/en-US.

Adjust the build infra to unbreak mobile/xul, possibly port fixes from mobile/android/base/ to embedding/android for consistency.
Also, pick up shared files from shared dir, and remove cruft from android.

Notably, that should leave us with 
- an l10n.ini in mobile/locales for the dashboard, covering all of xul and android
- an l10n.ini in mobile/android/locales and mobile/xul/locales, each for the corresponding builds and merge-% make targets.
- three same versions of filter.py for each of those.

Pros: 
- The files that developers on android care about don't move again.
- The files that are administrative hard are in one spot, notably, searchplugins
- The overload files which need to correspond to the toolkit versions are going to be fine for xul free of cost, by having them shared with android.
- No change to the entry points that releng uses to build/release android as platform.

Cons: More back and forth between build magic, even though they should be transparent to build automation.

Comments?

Comment 1

[Doug Turner (:dougt)]

removing the cruft can be done in a separate bug(s).

> Move searchplugins, profile, and chrome/overrides back to mobile/locales/en-US.

Does this not assume that both the xul ui and the native ui will have exactly the same searchplugins, bookmark strings?

The overrides seem reasonable since both the xul ui and native ui will need these strings.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Doug Turner (:dougt) from comment #1)
> removing the cruft can be done in a separate bug(s).
> 
> > Move searchplugins, profile, and chrome/overrides back to mobile/locales/en-US.
> 
> Does this not assume that both the xul ui and the native ui will have
> exactly the same searchplugins, bookmark strings?

searchplugins and bookmark strings seem reasonable to share.

> The overrides seem reasonable since both the xul ui and native ui will need
> these strings.

As long as the overrides are limited to in-content UI, this should be OK.

FWIW, we will be giving first priority to native Fennec in any of these shared cases.

Comment 3

[chris hofmann]

i'd suggest bumping up the priority of this. if we are going to do it we should do it now. the value of this decreases over time.

Comment 4

[Axel Hecht [:Pike]]

I'm going to do this now, based on our discussion on the l10n call.

The final patch includes porting the decision from bug 708437 about the dependency stuff there.

Comment 5

[Axel Hecht [:Pike]]

Attachment: make both xul and android l10n work together

This patch looks a bit unwieldy, but it's not so bad.

1) port the make file foo that I did for android to xul, that's most of what's going on in embedding/android. Might need to port bug 708437, too, if that turns out to be needed.

2) create l10n.inis in mobile/locales, mobile/android/locales, mobile/xul/locales. The first is the superset of the two others.

3) have filter.py's next to each, bitwise identical

4) migrate search (including region.properties, which is just search meta data), overrides, and profile to mobile/locales

5) factor the toolkit and 4) logic into mobile/locales/Makefile.in

6) remove installer

7) remove updater and crashreporter from android. Both are only used for desktop builds, aka, only the xul ui.

8) folded android/base into android l10n.ini-wise. embedding/android could have been an independent module used by other apps, mobile/android/base is not. Thus no need to externalize an l10n.ini for that. cosmetics, really.

Comment 6

[Axel Hecht [:Pike]]

FYI: Tested both xul and android on both multi and single locale builds, uploaded to my android tablets.

Comment 7

[Axel Hecht [:Pike]]

Comment on attachment 581792 [details] [diff] [review]
make both xul and android l10n work together

stas, can you start with a review on the l10n.ini and filter.py side and if the location of the files make sense from a localizer point of view?

Comment 8

[Aki Sasaki (not active)]

(In reply to Axel Hecht [:Pike] from comment #5)
> 7) remove updater and crashreporter from android. Both are only used for
> desktop builds, aka, only the xul ui.

Does this mean only removing the l10n strings for these?

We will want to be able to update android native from AUS, aiui, so we shouldn't remove that ability.

However, if the UI+strings are not there but we're still able to update the apk, we're fine.

Comment 9

[Axel Hecht [:Pike]]

(In reply to Aki Sasaki [:aki] from comment #8)
> (In reply to Axel Hecht [:Pike] from comment #5)
> > 7) remove updater and crashreporter from android. Both are only used for
> > desktop builds, aka, only the xul ui.
> 
> Does this mean only removing the l10n strings for these?
> 
> We will want to be able to update android native from AUS, aiui, so we
> shouldn't remove that ability.
> 
> However, if the UI+strings are not there but we're still able to update the
> apk, we're fine.

The strings are only used in the native-UI progress dialogs, which we have for the desktop platforms. There's no such thing for android, thus the strings aren't used.

Comment 10

[Staś Małolepszy :stas]

Comment on attachment 581792 [details] [diff] [review]
make both xul and android l10n work together

I like this a lot.  The localizers will end up with the following directory structure in their repositories:

  mobile/android
  mobile/android/base
  mobile/chrome (for region.properties)
  mobile/overrides
  mobile/profile
  mobile/searchplugins
  mobile/xul

This seems to be logical and future-proof for when we switch off xul or add more platforms under mobile.

I'll work on instructions (and a shell script) for localizers on how to update their repositories after this lands.

Comment 11

[Axel Hecht [:Pike]]

stas actually still found a comment, I forgot to remove the obsolete files in mobile/xul/locales/en-US, 

wokbok-2:en-US axelhecht$ hg rm profile/ searchplugins/ chrome/overrides/ chrome/region.properties 
Entferne chrome/overrides/appstrings.properties
Entferne chrome/overrides/netError.dtd
Entferne chrome/overrides/passwordmgr.properties
Entferne profile/bookmarks.inc
Entferne searchplugins/amazondotcom.xml
Entferne searchplugins/google.xml
Entferne searchplugins/list.txt
Entferne searchplugins/twitter.xml
Entferne searchplugins/wikipedia.xml

I'm having that as a second patch in my queue now and have submitted that to try:

https://tbpl.mozilla.org/?tree=Try&rev=eb4c53d9e9bb

Comment 12

[Axel Hecht [:Pike]]

Attachment: fix up xul, too

This is a patch that's needed on top. Apparently my local tests forgot to do a clobber build at some point, and I didn't pick up a details between branding and the java app dir.

So in addition to the xul removes in this patch, I explicitly call into MOZ_BRANDING_DIRECTORY when I create the strings.xml for the xul ui in embedding/android/locales.
For the android ui, that's done by first doing the branding dir and then the app dir, where the java ui is now. embedding is rather early in toolkit though.

Requesting reviews from doug, as per irc earlier.

I've pushed the stack of patches to try again, https://tbpl.mozilla.org/?tree=Try&rev=9d2a18b19293, but I'll head to bed now.

Comment 13

[Doug Turner (:dougt)]

Comment on attachment 583044 [details] [diff] [review]
fix up xul, too

Review of attachment 583044 [details] [diff] [review]:
-----------------------------------------------------------------

i am assuming most of these are copy and paste.

::: mobile/xul/locales/en-US/chrome/overrides/appstrings.properties
@@ -14,5 @@
> -# The Original Code is mozilla.org code.
> -#
> -# The Initial Developer of the Original Code is
> -# Netscape Communications Corporation.
> -# Portions created by the Initial Developer are Copyright (C) 1998

2011?

@@ -17,5 @@
> -# Netscape Communications Corporation.
> -# Portions created by the Initial Developer are Copyright (C) 1998
> -# the Initial Developer. All Rights Reserved.
> -#
> -# Contributor(s):

your name.  mozilla <3 you.

Comment 14

[chris hofmann]

> -# The Original Code is mozilla.org code.
> -#
> -# The Initial Developer of the Original Code is
> -# Netscape Communications Corporation.
> -# Portions created by the Initial Developer are Copyright (C) 1998

 probably should remain 1998 if this is really needed at all anymore,
 in places like mobile/xul/ which did not exist unitl only recently.
 
 I don't recall AOL claiming any copyright, or copywrite extention, post the 
 merger round 1999.   

 if this is widespread gerv or someone like that ought to have a look and set up some guidelines about where and how its used.

Comment 15

[Axel Hecht [:Pike]]

You do realize that you're talking about a file I completely remove? ;-)

Comment 16

[Gervase Markham [:gerv]]

I wouldn't worry about those headers now anyway; the MPL 2 header replaces all that information.

Gerv

Comment 17

[Axel Hecht [:Pike]]

This is now on inbound, https://hg.mozilla.org/integration/mozilla-inbound/rev/b230ea62de17, and looking good.

Stas, can you send out our messaging to the newsgroup?

Comment 18

[Staś Małolepszy :stas]

(In reply to Axel Hecht [:Pike] from comment #17) 
> Stas, can you send out our messaging to the newsgroup?

Done.

Comment 19

[Ed Morley [:emorley]]

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

Comment 20

[Axel Hecht [:Pike]]

Attachment: previous patches in one, as landed

Carrying over reviews by stas and doug for the patch that actually landed.

Requesting approval to transplant b230ea62de17 onto aurora, we'll need that to localize Fennec 11.

Comment 21

[Alex Keybl [:akeybl]]

Comment on attachment 584176 [details] [diff] [review]
previous patches in one, as landed

[Triage Comment]
Approved for Aurora.

Comment 22

[Axel Hecht [:Pike]]

Landed on aurora, too: https://hg.mozilla.org/releases/mozilla-aurora/rev/9d9046c6cc6d

Comment 23

[Axel Hecht [:Pike]]

Sadly that transplant didn't remove xul/locales/en-US/searchplugins or /profile. WTH.

Pushing a fix to try now, https://tbpl.mozilla.org/?tree=Try&rev=b3a766723274.