The default bug view has changed. See this FAQ.

Search engines in fennec language packs don't work, search service doesn't unpack jar:jar:

VERIFIED FIXED

Status

()

Firefox
Search
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: xti, Assigned: vingtetun)

Tracking

({regression})

4.0 Branch
ARM
Android
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox5- affected)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
Build id : Mozilla/5.0 (Android;Linux armv7l;rv:2.2a1pre)Gecko/20110328
Firefox/4.2a1pre Fennec /4.1a1pre
Device: Sony Ericsson Xperia X10
OS: Android 2.1 update 1

Steps to reproduce:
1. Open Fennec App
2. Tap URL Bar
3. Tap on Magnifying Glass button
4. Install the arabic plugin by clicking on the first .xpi link (fennec-5.1a1pre.ar.langpack.xpi): http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/latest-mozilla-central-linux-l10n/ 
6. Repeat step 2 and 3.

Expected result:
After step 3 and 6 there will be displayed the list of default installed search engines.

Actual result:
Unable to tap on Magnifying Glass button because is grayed out. This is due to the fact that the search engines are not translated into arabic.

Comment 1

6 years ago
I reproduced this on the mac desktop build, with the arab and the german langpack.

This has nothing to do with the langpack itself, the list.txt and the xml files are in there. Also, there's an error message about chrome/ar.manifest not being found, but I fixed that locally and it doesn't have an effect.

I get tons of "Invalid chrome URI: /" in the error console.

If I turn on search logging, restart, I get

_findJAREngines: failed to get chromeFile for chrome://browser/locale/searchplugins/: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface]"  nsresult: "0x80004002 (NS_NOINTERFACE)"  location: "JS frame :: file:///Users/axelhecht/Downloads/Fennec.app/Contents/MacOS/components/nsSearchService.js :: <TOP_LEVEL> :: line 2822"  data: no]

That's http://mxr.mozilla.org/mozilla-central/source/toolkit/components/search/nsSearchService.js#2873.

Why? No idea. Gavin, do you?
Summary: Search engines are not translated into arabic → Search engines in language packs don't work
Cristian - Have you tested other locales? I am wondering if this happens everywhere, or just in Arabic.

Comment 3

6 years ago
I tested with the german langpack, too.

chrome://browser/locale/searchplugins/ shows the german content allright then.

Comment 4

6 years ago
For the localizations present in the multilanguage build in Preferences, the search engines work and also are personalized (each language has the one mostly used in those country)
Created attachment 522651 [details] [diff] [review]
Patch

Per IRC this happen at least on Arabic, German and French, and this probably happens for each langs :(
Assignee: nobody → 21
Attachment #522651 - Flags: review?

Comment 6

6 years ago
This is likely a regression from not unpacking .xpi's anymore when installing add-ons.

Thus we end up with double jar urls, and the search service unpacks only once.

Moving to Firefox:Search, as there doesn't seem to be a toolkit search component.
Assignee: 21 → nobody
Component: General → Search
Keywords: regression
Product: Fennec → Firefox
QA Contact: general → search
Summary: Search engines in language packs don't work → Search engines in fennec language packs don't work, search service doesn't unpack jar:jar:
Version: Trunk → 4.0 Branch

Updated

6 years ago
Attachment #522651 - Flags: review? → review?(gavin.sharp)
Hmm, I want to block fennec 4.0.1 on this bug, but can't set the flag here
Oh, I just read comment 4 and I don't think we should block Fennec 4.0.1 on this. We'll take the patch when it's ready though.
Blocks: 533038
Blocks: 601018
Comment on attachment 522651 [details] [diff] [review]
Patch

I wish we had tests :(
Attachment #522651 - Flags: review?(gavin.sharp) → review+
Actually I suppose it would be relatively easy to test this with an xpcshell test.
Vivien, can you try to make a test? I'd like to get this in FF5 for the l10n changes we have planned.
Created attachment 525703 [details] [diff] [review]
Patch + test

Same patch with a simple test.
Assignee: nobody → 21
Attachment #525703 - Flags: review?
Comment on attachment 525703 [details] [diff] [review]
Patch + test

Oups, I've tried :gavin as the reviewer.... Sorry for spamming.
Attachment #525703 - Flags: review? → review?(gavin.sharp)
Created attachment 525716 [details] [diff] [review]
Patch + test (v0.2)

From irc comments, the patch with more files (and a fix)
Attachment #525703 - Attachment is obsolete: true
Attachment #525703 - Flags: review?(gavin.sharp)
Attachment #525716 - Flags: review?(gavin.sharp)
Comment on attachment 525716 [details] [diff] [review]
Patch + test (v0.2)

Looks good, but this test fails in Firefox for a couple of reasons:
- the test also needs to set gPrefService.setBoolPref("browser.search.loadFromJars", true);
- the Firefox searchengine defaults get loaded, so there end up being 7 engines instead of 2 (the duplicate google.xml in the JAR is not loaded)

You can address the latter by just checking that the JAR engine loaded (using e.g. searchService.getEngineByName()). Probably also would be good to remove the duplicate google.xml in the test JAR, rename the JAR to searchTest.jar rather than ar.jar, and rename the engine inside it to something unique (e.g. "Test for bug 645970" rather than "wikipedia-ar").
Attachment #525716 - Flags: review?(gavin.sharp) → review-
Created attachment 525726 [details] [diff] [review]
Patch + test (v0.3)
Attachment #522651 - Attachment is obsolete: true
Attachment #525716 - Attachment is obsolete: true
Attachment #525726 - Flags: review?(gavin.sharp)
Comment on attachment 525726 [details] [diff] [review]
Patch + test (v0.3)

>diff --git a/toolkit/components/search/tests/xpcshell/test_645970.js b/toolkit/components/search/tests/xpcshell/test_645970.js

>+function run_test() {

>+  // Assign our new registered chrome uri as the Naboo's planet

I do not understand this comment - remove it? :)
Attachment #525726 - Flags: review?(gavin.sharp) → review+
Duplicate of this bug: 643048
(In reply to comment #17)
> Comment on attachment 525726 [details] [diff] [review]
> Patch + test (v0.3)
> 
> >diff --git a/toolkit/components/search/tests/xpcshell/test_645970.js b/toolkit/components/search/tests/xpcshell/test_645970.js
> 
> >+function run_test() {
> 
> >+  // Assign our new registered chrome uri as the Naboo's planet
> 
> I do not understand this comment - remove it? :)

http://www.starwars.com/databank/character/jarjarbinks/ :)

But I can remove it if needed :)
http://hg.mozilla.org/mozilla-central/rev/15b8b01a965e

I have removed the comment, poor jarjar!
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Comment on attachment 525726 [details] [diff] [review]
Patch + test (v0.3)

>diff --git a/toolkit/components/search/tests/Makefile.in b/toolkit/components/search/tests/Makefile.in
>new file mode 100644
>--- /dev/null
>+++ b/toolkit/components/search/tests/Makefile.in
>@@ -0,0 +1,49 @@
>+# ***** BEGIN LICENSE BLOCK *****
>+# Version: MPL 1.1/GPL 2.0/LGPL 2.1
>+#
>+# The contents of this file are subject to the Mozilla Public License Version
>+# 1.1 (the "License"); you may not use this file except in compliance with
>+# the License. You may obtain a copy of the License at
>+# http://www.mozilla.org/MPL/
>+#
>+# Software distributed under the License is distributed on an "AS IS" basis,
>+# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
>+# for the specific language governing rights and limitations under the
>+# License.
>+#
>+# The Original Code is mozilla.org code.
>+#
>+# The Initial Developer of the Original Code is
>+# Mozilla.org.

"Mozilla.org" doesn't exist. This should be "the Mozilla Foundation".

>+# Portions created by the Initial Developer are Copyright (C) 2005

2005?

>new file mode 100644
>--- /dev/null
>+++ b/toolkit/components/search/tests/xpcshell/head_search.js
>new file mode 100644
>--- /dev/null
>+++ b/toolkit/components/search/tests/xpcshell/test_645970.js
>@@ -0,0 +1,63 @@
>+ * The Initial Developer of the Original Code is POTI Inc.
>+ * Portions created by the Initial Developer are Copyright (C) 2007
>+ * the Initial Developer. All Rights Reserved.

Really?
Verified using https://hg.mozilla.org/mozilla-central/rev/ec809c159ad2 Mozilla/5.0 (X11; Linux x86_64; rv:6.0a1) Gecko/20110415 Firefox/6.0a1 Fennec/6.0a1
Status: RESOLVED → VERIFIED

Comment 23

6 years ago
This affects Firefox mobile 5 beta. Should we fix this on beta as well or just let go till Firefox mobile 6?
status-firefox5: --- → affected
tracking-firefox5: --- → ?

Comment 24

6 years ago
Thomas, does this block fx mobile 5?
Not blocking Beta for me.
I added this bug to "Known Issues" in our Beta rel notes
Do you want us to track this for 5 final or not?  We don't know if you're OK during beta as well as final or only beta?

Comment 28

6 years ago
Not tracked for fennec5, so not for trunk either.
tracking-firefox5: ? → -
You need to log in before you can comment on or make changes to this bug.