937769 - JS Error: Error in invoking addEngine, no onError

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

E/GeckoConsole(19787): [JavaScript Error: "Error invoking addEngine install callback: [Exception... "'JavaScript component does not have a method named: "onError"' when calling method: [nsISearchInstallCallback::onError]"  nsresult: "0x80570030 (NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED)"  location: "JS frame :: jar:jar:file:///data/app/org.mozilla.fennec-2.apk!/assets/omni.ja!/components/nsSearchService.js :: SRCH_SVC_addEngine/engine._installCallback :: line 3870"  data: no]" {file: "jar:jar:file:///data/app/org.mozilla.fennec-2.apk!/assets/omni.ja!/components/nsSearchService.js" line: 3872}]

I attmpted to add the BMO engine. Nothing happened. It failed.

Comment 1

[Chenxia Liu [:liuche]]

Thanks for catching this, I somehow missed this part of the interface...This is a pretty quick little fix.

Comment 2

[Chenxia Liu [:liuche]]

Attachment: Patch: implement onError in nsSearchInstallCallback

Didn't fully implement the nsSearchInstallCallback interface originally. Added some strings.

Ian, any thoughts on the strings?
For an unknown error: "Couldn't add <engine-name> as a search engine"
Duplicate engine already exists: "<engine-name> is already one of your search engines"

Comment 3

[Chenxia Liu [:liuche]]

I wasn't able to reproduce an "unknown error," but it's possible to create a "duplicate engine" error by opening copies of the same page (that supports opensearch) in two tabs, and then trying to add the engine for each tab.

Relatedly: bug 937870 always displays a "duplicate engines" prompt, which can be observed in the above STR.

Comment 4

[Lucas Rocha (:lucasr)]

Comment on attachment 831093 [details] [diff] [review]
Patch: implement onError in nsSearchInstallCallback

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

Looks good. Suggested changes are optional.

::: mobile/android/chrome/content/browser.js
@@ +6808,5 @@
> +            NativeWindow.toast.show(Strings.browser.formatStringFromName("alertSearchEngineErrorToast", [engine.title], 1), "long");
> +            break;
> +          case 2:
> +            // Engine is a duplicate.
> +            NativeWindow.toast.show(Strings.browser.formatStringFromName("alertSearchEngineDuplicateToast", [engine.title], 1), "long");

Maybe something like this would be cleaner?

let errorMessage;
if (aCode == 2) {
    // Engine is a duplicate.
    errorMessage = "alertSearchEngineDuplicateToast";
} else {
    // Unknown failure. Display general error message.
    errorMessage = "alertSearchEngineErrorToast";
}

if (errorMessage) {
    NativeWindow.toast.show(Strings.browser.formatStringFromName(errorMessage, [engine.title], 1), "long");
}

Not a big deal though. Up to you. In any case, maybe we should always an error message if onError is called?

Comment 5

[Ian Barlow (:ibarlow)]

(In reply to Chenxia Liu [:liuche] from comment #2)
> Created attachment 831093 [details] [diff] [review]
> Patch: implement onError in nsSearchInstallCallback
> 
> Didn't fully implement the nsSearchInstallCallback interface originally.
> Added some strings.
> 
> Ian, any thoughts on the strings?
> For an unknown error: "Couldn't add <engine-name> as a search engine"
> Duplicate engine already exists: "<engine-name> is already one of your
> search engines"

that sounds good to me, Chenxia

Comment 6

[Chenxia Liu [:liuche]]

Attachment: Patch: implement onError in nsSearchInstallCallback v2

Carrying over r+.

Will land once trees are open again...

Comment 7

[Chenxia Liu [:liuche]]

https://hg.mozilla.org/integration/fx-team/rev/02cc61730c27

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/02cc61730c27

Comment 9

[Flaviu Cos]

Verified as fixed in build: Nightly 28.0a1 (2013-11-19)
Device: Asus Transformer Tab (Android 4.0.3)
Note: The BMO search engine is successfully added without any errors in the logcat.