Closed Bug 334486 Opened 14 years ago Closed 14 years ago

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

Categories

(Firefox :: Search, defect, P1)

2.0 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 2 beta2

People

(Reporter: Gavin, Assigned: mwu)

References

Details

(Keywords: fixed1.8.1, late-l10n)

Attachments

(2 files, 2 obsolete files)

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.
Status: NEW → ASSIGNED
Flags: blocking-firefox2?
Priority: -- → P1
Attached patch wip (obsolete) — Splinter Review
just dumping this here for now
Flags: blocking-firefox2? → blocking-firefox2+
Blocks: 335435
Retargetted at beta1
Target Milestone: Firefox 2 alpha2 → Firefox 2 beta1
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...
Keywords: uiwanted
(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)
(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).
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?
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"
Whiteboard: 181b1+
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?
Assignee: gavin.sharp → michael.wu
Status: ASSIGNED → NEW
Seems to work okay, though there's some storage assertion during the restart.
Attachment #218817 - Attachment is obsolete: true
Attachment #228110 - Flags: review?(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.
Attachment #228110 - Flags: ui-review?(beltzner)
Attachment #228110 - Flags: review?(mconnor)
Attachment #228110 - Flags: review+
Attachment #228110 - Flags: ui-review?(beltzner) → ui-review+
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?
Whiteboard: 181b1+ → 181b1+ [checkin needed]
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
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: late-l10n
Resolution: --- → FIXED
Summary: Ability to restore shipped search engines after they've been removed → Ability to restore shipped search engines from the safe mode dialog, after they've been removed
Whiteboard: 181b1+ [checkin needed] → 181b1+
Attached patch as checked inSplinter Review
Attachment #228110 - Attachment is obsolete: true
As you can see from the interdiff, I made the patch use the _isInAppDir attribute that was added in bug 335460.
Whiteboard: 181b1+ → [need-a] 181b1+
Attachment #228160 - Flags: approval1.8.1?
--> beta2, not needed for beta1, and might as well get more testing
Target Milestone: Firefox 2 beta1 → Firefox 2 beta2
Blocks: 343707
Opened bug 343707 for adding this feature to the search manager.
Blocks: 286589
Whiteboard: [need-a] 181b1+ → [need-a]
Approving this, Axel, there's late l10n here for you ...
Attachment #228160 - Flags: approval1.8.1? → approval1.8.1+
Keywords: fixed1.8.1
Whiteboard: [need-a]
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
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.
Status: RESOLVED → REOPENED
Keywords: fixed1.8.1qawanted
Resolution: FIXED → ---
Part of the problem seems to be that the part of the patch that changed nsSearchService.js never got checked in on branch.
This is the part of the patch that didn't make it into branch when the rest of the patch landed.
Whiteboard: [checkin needed (1.8 branch)]
mozilla/browser/components/search/nsSearchService.js 	1.1.2.52
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [checkin needed (1.8 branch)]
Blocks: 348737
You need to log in before you can comment on or make changes to this bug.