Created attachment 603755 [details] [diff] [review] Various fixes for webSearchProvider.js I'm trying to get the web search feature working in Ubuntu, and hit some issues with webSearchProvider.js. - "Cu" isn't defined anywhere, which prevents the component from loading at all - The definition of NS_XPCOM_CURRENT_PROCESS_DIR has a typo - The logic in appendFileKey() for checking if the location exists is reversed - The category stanza is incorrect in mailComponents.manifest, as nsDirectoryService::RegisterCategoryProviders() doesn't understand "service,@mozilla.org/mail/web-search-provider;1". This is specific to nsAppStartupNotifier, which handles the "app-startup" category. - getFiles() returns non-existant searchplugin directories for extensions (even packed extensions), which makes nsSearchService blow up This doesn't affect mozilla.org builds, as the searchplugin location in the application directory is found by the default provider. In Ubuntu, we want to install the searchplugins in to distribution/searchplugins, which gives us the ability to have language packs provide localized plugins
I should clarify - by "doesn't affect mozilla.org builds", I mean that the opensearch stuff still works by virtue of webSearchProvider.js not loading at all, but the bugs aren't specific to our builds :)
Comment on attachment 603755 [details] [diff] [review] Various fixes for webSearchProvider.js Review of attachment 603755 [details] [diff] [review]: ----------------------------------------------------------------- Ok, this looks great and I've verified all the variations, thanks for the patch.
Comment on attachment 603755 [details] [diff] [review] Various fixes for webSearchProvider.js [Triage Comment] We should take this on aurora & beta as well, maybe esr - it fixes something that is broken and it'll provide greater flexibility for extensions, distributions etc.
Checked in: http://hg.mozilla.org/releases/comm-aurora/rev/7368ce5fc056 http://hg.mozilla.org/releases/comm-beta/rev/491ada0b739c
I had to push a unit test bustage fix for this. Now the file is actually working, xpconnect didn't like the throw Components.results.NS_ERROR_NOT_IMPLEMENTED; in getFile() - it would log an error to the console and that was upsetting some of the tests that relied on no errors being logged. Therefore I changed this just to return null which still is handled correctly but avoids the warning: http://hg.mozilla.org/comm-central/rev/c20698efdd88 http://hg.mozilla.org/releases/comm-aurora/rev/eb3c266e224e http://hg.mozilla.org/releases/comm-beta/rev/37248c37f757
Unfortunately I had to back this out due to continuing unit test failures: http://hg.mozilla.org/comm-central/rev/15769b1a16d0 http://hg.mozilla.org/releases/comm-aurora/rev/c87d0eee72a8 http://hg.mozilla.org/releases/comm-beta/rev/83fc75101b40
Oh, my. Brace yourself for this :) So, the test failures aren't caused by any errors in webSearchProvider.js. It actually turns out that the exceptions that are seen in the test failures occurred previously. Eg, here, nsMsgDBFolder::GetStringProperty() returns NS_MSG_ERROR_FOLDER_MISSING: TEST-UNEXPECTED-FAIL | /buildbot/comm-central-linux64-opt-unittest-xpcshell/build/xpcshell/tests/mailnews/db/gloda/test/unit/test_index_messages_imap_offline.js | test failed (with xpcshell return code: 0), see following log: TEST-UNEXPECTED-FAIL | ../../../../resources/logHelper.js | Error console says [stackFrame [Exception... "Component returned failure code: 0x80550007 [nsIMsgFolder.getStringProperty]" nsresult: "0x80550007 (<unknown>)" location: "JS frame :: resource:///modules/gloda/datastore.js :: gloda_ds_mapFolderURI :: line 2084" data: no]] - See following stack: This is propagated as an exception in JS all the way back to xpconnect, and happened even without my change. What normally happens is that nsXPCWrappedJSClass::CheckForException() delivers this error to the error reporter registered by xpcshell (my_ErrorReporter()). The error is logged by xpcshell and then basically swallowed and goes no further. With my change committed, the error is no longer being handled by the xpcshell error handler, because the error handler hasn't been set correctly on the JSContext. This means that xpconnect delivers it to the console service instead, where it is caught by the test harness --> fail. The reason this is happening is because when xpcshell starts xpcom, nsDirectoryService loads the providers registered using the xpcom-directory-providers category. Now we are registering a provider implemented in JS, this is causing the JSContext to be created (in mozJSComponentLoader::ReallyInit()) *before* xpcshell has had a chance to set up its context callback (using JS_SetContextCallback), which it uses to override the error reporter on newly created JSContext's.
So, I'm not sure how to proceed here, other than "rewrite webSearchProvider.js" in C++ ;)
(In reply to Chris Coulson from comment #8) > The error is logged by > xpcshell and then basically swallowed and goes no further. > What I meant to say here is that it is logged by xpcshell and not to the console service. Of course, the error still propagates through xpconnect to native code
I'm not sure how I managed to change all of the status flags, but that wasn't intentional :/
Comment on attachment 612484 [details] [diff] [review] Switch to C++ Review of attachment 612484 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me!
Comment on attachment 612484 [details] [diff] [review] Switch to C++ gloda tests are a bit flaky, but I think this is OK.
Comment on attachment 612484 [details] [diff] [review] Switch to C++ [Triage Comment]
Checked in: http://hg.mozilla.org/releases/comm-aurora/rev/400d35483957 http://hg.mozilla.org/releases/comm-beta/rev/8db3dc572a19