localized search engines for maemo multi-locale build

RESOLVED FIXED in fennec1.0b5

Status

RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Pike, Assigned: Gavin)

Tracking

Trunk
fennec1.0b5
ARM
Maemo
Dependency tree / graph

Details

(Whiteboard: [fennec l10n])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

9 years ago
There are two pieces we need for getting localized search engines to work in a multi-locale build:

- packaging

- hooking the search engines up to the service.

For the packaging, this should hook up to the chrome-% target from bug 519682. There is one nifty detail to pay attention to when doing fallback: search plugins should be taken from en-US, and fallback to l10n. list.txt should come from l10n, and if LOCALE_MERGEDIR is defined, fallback to en-US.

I'll leave it up to gavin to lay out how to get the search engine service stuff done, he knows that much better than I. Looking at our startup process, the later the merrier.

TBD: We do launch fennec once on install on hildon. I'm not sure if we need to bother about the scenario when a user installs fennec, switches device locale, and then starts up fennec for the first time (in her opinion). If we can get that for free, that'd be great. Might be that users are so keen to have their Firefox on their device that they do that first, and then switch figure to switch the device's language.
(Reporter)

Updated

9 years ago
Depends on: 519682
Whiteboard: [fennec l10n]
(Reporter)

Updated

9 years ago
tracking-fennec: --- → ?
The search service changes are pretty straightforward - I'm having more trouble figuring out how to do the packaging.

I think we want to package the engines in the locale JAR like we do for bookmarks, but I'm not sure what the best way to do that is. JarMaker seems to be pretty reliant on JAR manifests, which I'm assuming we don't want to use in this case (since we already have list.txt and the associated build-merge magic).

I guess we should just write a separate script that does simple jar file adding or extend JarMaker... thoughts on which is a better option?
(Reporter)

Comment 2

9 years ago
Actually, I think we should just pipe something in from stdin or a temp file to JarMaker.py.

libs realchrome:: $(addsuffix .xml, $(SEARCH_PLUGINS))
... and then with tabs
@echo "$(AB_CD).jar:" > tmp-search.jar
@for line in $(foreach plugin,$^,"  locale/$(AB_CD)/browser/searchplugins/$(notdir $(plugin)) ($(plugin))");do echo $$line >> tmp-search.jar;done
# possibly add list.txt, too. I guess you want that?
# and now call in to JarMaker.py explicitly, you probably can go without many of the flags, as the substitution should be mostly dealt with

makes sense?
Created attachment 406832 [details] [diff] [review]
fennec WIP

That works, with a few modifications as discussed on IRC - thanks for the tip!
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Created attachment 406833 [details] [diff] [review]
search service WIP

These are the changes required to allow loading search engines from a JAR file. It loads them synchronously, which isn't ideal but is kind of required given the expectations of search service consumers.

Fennec needs to set the prefs from the patch to indicate which packages engines should be loaded from, since there's no way to enumerate locale packages, packaging the files in the global locale JAR would be tricky, and we don't want to hardcode the /browser/ package in toolkit's search service.
Depends on: 523453
Moved the search service patch over to bug 523453.

Updated

9 years ago
tracking-fennec: ? → 1.0+
Created attachment 408190 [details] [diff] [review]
fennec patch
Attachment #406832 - Attachment is obsolete: true
Attachment #406833 - Attachment is obsolete: true
Attachment #408190 - Flags: review?(mark.finkle)
Attachment #408190 - Flags: review?(l10n)
Comment on attachment 408190 [details] [diff] [review]
fennec patch

> clobber-zip:
> 	$(RM) $(STAGEDIST)/chrome/$(AB_CD).jar \
> 	  $(STAGEDIST)/chrome/$(AB_CD).manifest \
> 	  $(STAGEDIST)/defaults/preferences/mobile-l10n.js
>-	$(RM) -r $(STAGEDIST)/searchplugins \
> 	  $(STAGEDIST)/dictionaries \
> 	  $(STAGEDIST)/defaults/profile \
> 	  $(STAGEDIST)/chrome/$(AB_CD)

This looks odd. Don't the next 3 lines need to be removed too? Or a line continuation added to the line above.

r+ with that answered/addressed
Attachment #408190 - Flags: review?(mark.finkle) → review+
Oh, huh, wonder how that even worked. Fixed locally:

clobber-zip:
	$(RM) $(STAGEDIST)/chrome/$(AB_CD).jar \
	  $(STAGEDIST)/chrome/$(AB_CD).manifest \
	  $(STAGEDIST)/defaults/preferences/mobile-l10n.js
	$(RM) -r $(STAGEDIST)/dictionaries \
	  $(STAGEDIST)/defaults/profile \
	  $(STAGEDIST)/chrome/$(AB_CD)
OS: Linux (embedded) → All
Hardware: ARM → All
Target Milestone: --- → B5
Pushed that, can address any nits from Axel ex post facto:

https://hg.mozilla.org/mobile-browser/rev/b7fed3434b45
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Reporter)

Updated

9 years ago
Attachment #408190 - Flags: review?(l10n) → review+
(Reporter)

Comment 10

9 years ago
Comment on attachment 408190 [details] [diff] [review]
fennec patch

Basically r+, but the $(RM) -r section for directories needs to stay a -r section. Not that we have a ton of that yet, but still, dictionaries, profile, and unjar'ed l10n chrome dirs need to be $(RM) -r.
(Reporter)

Comment 12

9 years ago
Ah right, I saw that comment flying by, but it didn't fully enter my head.

Sounds like all things are in check then, cool.

Updated

8 years ago
Component: Linux/Maemo → General
OS: All → Linux (embedded)
QA Contact: maemo-linux → general
Hardware: All → ARM
You need to log in before you can comment on or make changes to this bug.