Closed Bug 438599 Opened 12 years ago Closed 5 years ago
Engine attribute of ns IBrowser Search Service should never be null
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-GB; rv:1.9) Gecko/2008052906 Firefox/3.0 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-GB; rv:1.9) Gecko/2008052906 Firefox/3.0 In Firefox 3.0 RC 2, the "View Selection Source" item in the right-click menu on a webpage (with some text selected) doesn't do anything. No selection source, not even an error message. Reproducible: Always
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008061105 Minefield/3.0pre This works fine for me. Have you tried: - Firefox's safe-mode to exclude extension/theme problems - a new profile - a reinstall in a new empty folder? http://kb.mozillazine.org/Safe_Mode_(Firefox) http://kb.mozillazine.org/Profile_Folder
Apparently continually upgrading from the Firefox 3 betas to the Release Candidate managed to leave the "searchplugins" sub-folder in the Firefox installation path empty. With any single file in that sub-folder, things work as expected. With the folder empty, two behaviours change: * right-click menu is sometimes ten times larger (contains every possible item from every possible right-click menu, including "View Image" even though I didn't right-click an image), sometimes the standard webpage right-click menu (which doesn't include "View Selection Source" or anything else context-specific). Couldn't find a pattern between the two. * "View Selection Source", if present, is dead.
Summary: "View Selection Source" completely dead → Empty searchplugins folder renders "View Selection Source" completely dead
Any error messages in the Error Console when this happens? Especially ones from browser.js or other chrome JS?
please retest with newer version. Close, or update comment/attach testcase as appropriate to test results. Thanks
Whiteboard: [closeme 2010-06-25]
Still happening exactly as described.
9.0a1 (2011-09-11): still happens. STR: - Get nightly with a clean profile - Delete everything from "searchplugins" - Run the nightly - Go to www.google.co.uk/search?q=asdf - Select some text - Right-click. Expected: a proper menu with Select all, Copy, Search (greyed out) and View Selection Source. Actual: Sometiems a menu about two screens tall with every single menu item there is. Other times a small menu with irrelevant items.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: View Source → General
QA Contact: view.source → general
Summary: Empty searchplugins folder renders "View Selection Source" completely dead → Empty searchplugins folder breaks the right-click menu
Version: 1.9.0 Branch → Trunk
The problem here is that the search service's default engine is null. Removing the files directly from the searchplugins folder won't be possible anymore now that bug 1162569 is fixed. Another way to reproduce is to remove all the search engines from the preferences UI, except the last one that the UI won't let you remove, and then from the browser console execute: Services.search.removeEngine(Services.search.defaultEngine) Then the context menu issue is that http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js?rev=e7875576b042#1742 breaks when ss.defaultEngine is null. I'm not sure we should fix it in the context menu code though, it could be better to ensure the search service never reaches the state of not having a default engine.
Morphing the bug a little bit so that it's about the solution that we need, rather than a specific instance of the problem. A faster way to reproduce is to execute this code: Services.search.getVisibleEngines().forEach(Services.search.removeEngine);
Assignee: nobody → florian
Component: General → Search
OS: Windows 2000 → All
Product: Toolkit → Firefox
Hardware: x86 → All
Summary: Empty searchplugins folder breaks the right-click menu → the currentEngine attribute of nsIBrowserSearchService should never be null
The original default engine should never be null (now that they are part of the omni.ja file, they are unlikely to ever be physically removed from the disk), so we don't need to have a built-in fallback engine, we can just unhide the original default, and the fix turns out to be very simple. https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea8988dcf4e4
Attachment #8656591 - Flags: review?(dtownsend)
Comment on attachment 8656591 [details] [diff] [review] Patch Review of attachment 8656591 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/search/tests/xpcshell/test_currentEngine_fallback.js @@ +20,5 @@ > + // Verify the original default engine is used as a fallback and no > + // longer hidden. > + do_check_eq(Services.search.currentEngine.name, currentEngine.name); > + do_check_false(currentEngine.hidden); > + do_check_eq(Services.search.getVisibleEngines().length, 1); This is a little weird API-wise, would it be better to just have removeEngine throw on the attempt to remove the last visible engine?
(In reply to Dave Townsend [:mossop] from comment #10) > This is a little weird API-wise, would it be better to just have > removeEngine throw on the attempt to remove the last visible engine? Not really. Downsides of that approach would be: - wouldn't fix existing broken profiles. (Telemetry shows we have a non-trivial number of users who have 'NONE' as the reported default engine) - would leave users with the last engine of the list set as the default, which is probably not an engine people would want to have (one may end up with twitter or ebay becoming the default for no good reason).
Attachment #8656591 - Flags: review?(dtownsend) → review+
You need to log in before you can comment on or make changes to this bug.