334486 - Ability to restore shipped search engines from the safe mode dialog, after they've been removed

Status: RESOLVED FIXED

(Firefox :: Search, defect, P1)

Description

[:Gavin Sharp [email: [e-mail]]]

The engine manager (bug 232272) needs to include a "reset to default" button that can be used to restore the search plugins that shipped with Firefox.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Attachment: wip

just dumping this here for now

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Retargetted at beta1

Comment 3

[:Gavin Sharp [email: [e-mail]]]

I'm not sure where this button belongs. Making it a button in the search engine manager is kinda complicated because of the way that dialog batches changes to be committed when you press "OK". Putting it in the pref panel isn't really discoverable, and it doesn't really fit into any of the existing pref panes. Any other suggestions?

If I do end up putting it in the engine manager dialog, the hard part is making it have immediate effect when clicked, since it invalidates all the previous changes made in the dialog. Maybe it could be a button along side OK and Cancel, which would dismiss the dialog and restore the engines? That seems kind of unusual...

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #3)
> Any other suggestions?

You're right in saying that it's not discoverable in the preferences panel, although even that's more discoverable than having it only be available in Safe Mode, so let's keep that option as a "worst case" scenario.

> If I do end up putting it in the engine manager dialog, the hard part is 
> making
> it have immediate effect when clicked, since it invalidates all the previous

Why is that a requirement? I would expect that if you clicked this button, it would restore the engines in the listbox, but if you then clicked "Cancel", it would not honour those changes. All changes that a user does in that panel would only take effect on clicking "OK".

(Speaking of which, "Get more search engines ..." should *probably* ask the user if changes should be applied first, but that might get fugly)

Comment 5

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #4)
> > If I do end up putting it in the engine manager dialog, the hard part is 
> > making it have immediate effect when clicked, since it invalidates all the
> > previous
> 
> Why is that a requirement? I would expect that if you clicked this button, it
> would restore the engines in the listbox, but if you then clicked "Cancel", it
> would not honour those changes. All changes that a user does in that panel
> would only take effect on clicking "OK".

By "having an immediate effect when clicked" I meant "having the engines reappear in the listbox immediately, but only be actually restored when clicking OK". I don't remember the exact details, but it got tricky when the "restore" button was clicked, since I then need to go through the list of "to be performed when OK is clicked" actions and remove the ones that affect the engines that are restored. It also messed with the ordering since engines are all ordered and maintain their order even when "removed" (hidden for the default plugins).

Comment 6

[:Gavin Sharp [email: [e-mail]]]

We really need to make a decision on where this function belongs and get strings in before the string freeze.

mconnor/beltzner: is a button in the safe mode dialog a suitable choice? That's probably too obscure. I might be able to implement a button in the search engine manager, but that involves a lot more work.

Either way, can we determine the string we want on this button and land it now? How about "Restore default search engines"? Is one button all we need?

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

I'd really rather have the button in the search manager, as I, too, think that it's awkward in Safe Mode only.

As for strings, the button in the Search Manager should be: "Restore Defaults"

the option in Safe Mode should be: "Restore default search engines"

Comment 8

[Mike Connor [:mconnor]]

I think we need to get the function into Safe Mode (it should be there either way) and figure out the implementation for Search Manager in b2, since time is short.  Gavin, can you get a patch ready doing that for tonight?

Comment 9

[Michael Wu [:mwu]]

Attachment: Add ability to restore default search engines

Seems to work okay, though there's some storage assertion during the restart.

Comment 10

[Mike Connor [:mconnor]]

Comment on attachment 228110 [details] [diff] [review]
Add ability to restore default search engines

r=me, though I'm thinking the conistent approach is to nuke the profile-dir searchplugins as well, like we do with bookmarks etc.  Let's see what beltzner thinks, I'm ok either way here.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Beltzner thinks it's OK. Calling Gavin for a quick checkin. Once done, could we get it baking for a build and request 1.8.1 approval?

Comment 12

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/base/content/safeMode.js 	1.8
mozilla/browser/base/content/safeMode.xul 	1.7
mozilla/browser/components/search/nsIBrowserSearchService.idl 	1.11
mozilla/browser/components/search/nsSearchService.js 	1.47
mozilla/browser/locales/en-US/chrome/browser/safeMode.dtd 	1.3

Comment 13

[:Gavin Sharp [email: [e-mail]]]

Attachment: as checked in

Comment 14

[:Gavin Sharp [email: [e-mail]]]

As you can see from the interdiff, I made the patch use the _isInAppDir attribute that was added in bug 335460.

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

--> beta2, not needed for beta1, and might as well get more testing

Comment 16

[Michael Wu [:mwu]]

Opened bug 343707 for adding this feature to the search manager.

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

Approving this, Axel, there's late l10n here for you ...

Comment 18

[Bryan]

This appears to be broken!  Launching "C:\Program Files\Firefox\firefox.exe" -safe-mode and clicking on "Restore Default Search Engines" and then clicking on Restart does not restore the default search engines.

BUILD: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060726 BonEcho/2.0b1 ID:2006072702

~B

Comment 19

[Mike Connor [:mconnor]]

Confirmed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060727 BonEcho/2.0b1

Possible this regressed, adding qawanted to help with the binary search.

Comment 20

[Michael Wu [:mwu]]

Part of the problem seems to be that the part of the patch that changed nsSearchService.js never got checked in on branch.

Comment 21

[Michael Wu [:mwu]]

Attachment: Missing part of branch checkin

This is the part of the patch that didn't make it into branch when the rest of the patch landed.

Comment 22

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/components/search/nsSearchService.js 	1.1.2.52