Closed Bug 519685 Opened 15 years ago Closed 15 years ago

localized search engines for maemo multi-locale build

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Maemo
defect
Not set
normal

Tracking

(fennec1.0+)

RESOLVED FIXED
fennec1.0b5
Tracking Status
fennec 1.0+ ---

People

(Reporter: Pike, Assigned: Gavin)

References

Details

(Whiteboard: [fennec l10n])

Attachments

(1 file, 2 obsolete files)

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.
Depends on: 519682
Whiteboard: [fennec l10n]
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?
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?
Attached patch fennec WIP (obsolete) — Splinter Review
That works, with a few modifications as discussed on IRC - thanks for the tip!
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Attached patch search service WIP (obsolete) — Splinter Review
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.
tracking-fennec: ? → 1.0+
Attached patch fennec patchSplinter Review
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
Closed: 15 years ago
Resolution: --- → FIXED
Attachment #408190 - Flags: review?(l10n) → review+
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.
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.
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.

Attachment

General

Created:
Updated:
Size: