Closed
Bug 708015
Opened 13 years ago
Closed 13 years ago
support both xul and android UI at the same time
Categories
(Firefox for Android Graveyard :: General, defect, P4)
Firefox for Android Graveyard
General
Tracking
(firefox11 fixed)
RESOLVED
FIXED
Firefox 12
Tracking | Status | |
---|---|---|
firefox11 | --- | fixed |
People
(Reporter: Pike, Assigned: Pike)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Pike
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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•13 years ago
|
||
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•13 years ago
|
||
(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.
Updated•13 years ago
|
Priority: -- → P4
Comment 3•13 years ago
|
||
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.
Assignee | ||
Comment 4•13 years ago
|
||
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.
Assignee | ||
Comment 5•13 years ago
|
||
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.
Assignee | ||
Comment 6•13 years ago
|
||
FYI: Tested both xul and android on both multi and single locale builds, uploaded to my android tablets.
Assignee | ||
Comment 7•13 years ago
|
||
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?
Attachment #581792 -
Flags: review?(stas)
Comment 8•13 years ago
|
||
(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.
Assignee | ||
Comment 9•13 years ago
|
||
(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•13 years ago
|
||
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.
Attachment #581792 -
Flags: review?(stas) → review+
Assignee | ||
Comment 11•13 years ago
|
||
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
Assignee | ||
Comment 12•13 years ago
|
||
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.
Attachment #583044 -
Flags: review?(doug.turner)
Assignee | ||
Updated•13 years ago
|
Attachment #581792 -
Flags: review?(doug.turner)
Comment 13•13 years ago
|
||
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.
Attachment #583044 -
Flags: review?(doug.turner) → review+
Updated•13 years ago
|
Attachment #581792 -
Flags: review?(doug.turner) → review+
Comment 14•13 years ago
|
||
> -# 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.
Assignee | ||
Comment 15•13 years ago
|
||
You do realize that you're talking about a file I completely remove? ;-)
Comment 16•13 years ago
|
||
I wouldn't worry about those headers now anyway; the MPL 2 header replaces all that information.
Gerv
Assignee | ||
Comment 17•13 years ago
|
||
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?
Assignee | ||
Updated•13 years ago
|
Target Milestone: --- → Firefox 12
Comment 18•13 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #17)
> Stas, can you send out our messaging to the newsgroup?
Done.
Comment 19•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 20•13 years ago
|
||
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.
Attachment #581792 -
Attachment is obsolete: true
Attachment #583044 -
Attachment is obsolete: true
Attachment #584176 -
Flags: review+
Attachment #584176 -
Flags: approval-mozilla-aurora?
Comment 21•13 years ago
|
||
Comment on attachment 584176 [details] [diff] [review]
previous patches in one, as landed
[Triage Comment]
Approved for Aurora.
Attachment #584176 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 22•13 years ago
|
||
Landed on aurora, too: https://hg.mozilla.org/releases/mozilla-aurora/rev/9d9046c6cc6d
status-firefox11:
--- → fixed
Assignee | ||
Comment 23•13 years ago
|
||
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.
Updated•4 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
•