User-Agent: Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7a) Gecko/20040102 Firebird/0.7+ There should be a GUI method implemented for the management of search engine extensions in the toolbar. It is a straightforward process to add new ones using the "Add New Engines" option in the drop-down menu, but there is presently no way to use the GUI to manage installed plugins. Currently, the files have to be removed manually from the searchplugins folder through the filesystem, which works fine but is a hassle. Instead of an "add engines" menuitem which goes directly to Mycroft, a GUI interface should be implemented, perhaps under "Web Features" which could have controls for adding (which would go to Mycroft), removing (would work like the extensions list - click on one and click remove to get rid of it). Finally, search engines should be stored in user profile, not in the browser root, so they persist if the browser is upgraded. Reproducible: Always Steps to Reproduce:

I think the interface should still be accessible from the search bar. Instead of "Add Engines..." it could be "Manage Engines..." which would pop up the same part of the Options GUI you described. Also, engines should generally be stored in the profile, but still allowing search plugins in the browser root allows for cross-profile search plugins. It would make sense to keep the default Google search in the browser root and let users add new searches to the profile. It would be nice if regular plugins were handled this way too, but that's another bug.

*** Bug 235852 has been marked as a duplicate of this bug. ***

I would like to suggest to maybe not have a dialog (or have a dialog and what I am proposing) but: 1. A context menu with the option to show the properties and the possibility to delete the search engine 2. The ability to drag and drop the search engines just as bookmarks in order to change their ordering The fact that engines should be stored in the profile is also discussed in Bug 235600

Now that there's an easy way to manage Themes and Extensions, it just begs to make Serach Engine list editable as well. Nominating for v1.0 Prog.

Sort! It's only natural to sort the engines as they appear in the list according to user prefs. If you have lots of engines -- this would be very welcome. As an alternative, just have it listed the engines in order of last use, it's anyway better than the order of last added. Installing engines to profile directory is very much needed for fastupgraders, I always forget to save the engines when I reinstall Firefox.

-> search component

if there is going to be a context menu, please don't forget to include "New Separator". This would make it easy to group search engines, like e.g. Google Yahoo --------------- ebay --------------- Dictionary.com Babelfish EN-NL Babelfish NL-EN Babelfish DU-NL Babelfish NL-DU --------------- Add engines.... In context menu there should also be a properties option allowing people to change the title of the engine

(In reply to comment #5) > Sort! Search engines are sorted in alpha order. (After restart for sure) That seems OK for now. Also, there are bug 232638 and bug 245558 that look more critical to me. Bug 232638 Comment #8 has logical suggestions to the way search engines have to be managed.

It would have been nice if "Firefox" would have given a reason to -1.0 "Firefox 1.0 goal" -> feature complete and stable. minussing this leaves it feature incomplete. I don't think a deadline date should be a reason to compromise, what is in branch now should work by the time we reach FF1.0 Milestone.

*** Bug 259761 has been marked as a duplicate of this bug. ***

With 1.0PR, after a engine is added to the search engines toolbar, there's no way to remove it. I think it's a serious issue, and 1.0 should have at least this much implemented.

I have written the backend to delete search engines. To keep it simple, removing search engines will be done with the delete key in the engine popup. No manager for now.

Maybe we need a simple way to add the default engines in case someone accidentally deletes one. The "add engines" now leads to www.mozdev.org which is from my point of view the wrong place to look. This should really be coming from UMO, like anything else (extensions/plugins/themes) have to WE TRUST. The less scattered (on the web) all "plugins" are, the more professional it looks. If needed I can file a bug for it.

at the very least it should work the same way auto-complete lists are managed ... you highlight an entry and hit shift+del ... making the search engines list behave slightly different from other lists seems to be the wrong direction ...

(In reply to comment #12) > I have written the backend to delete search engines. To keep it simple, removing > search engines will be done with the delete key in the engine popup. > No manager for now. Pierre, with your patch, does the user get a popup ? |-------------------------------------------| | X | | Do you want to delete this searchengine ? | | | | [yes] [no] | |-------------------------------------------| I think that would be the minimally required UI ;-) if that is the case I'd be tempted to ?1.0 since we have a patch.

*** Bug 262958 has been marked as a duplicate of this bug. ***

*** Bug 261677 has been marked as a duplicate of this bug. ***

I think something should be done for 1.0 milestone. It's the sort of thing reviewers will pick up on and it's so trivial.

(In reply to comment #1) > I think the interface should still be accessible from the search bar. Instead of > "Add Engines..." it could be "Manage Engines..." which would pop up the same > part of the Options GUI you described. > > Also, engines should generally be stored in the profile, but still allowing > search plugins in the browser root allows for cross-profile search plugins. It > would make sense to keep the default Google search in the browser root and let > users add new searches to the profile. It would be nice if regular plugins were > handled this way too, but that's another bug. (In reply to comment #1) > I think the interface should still be accessible from the search bar. Instead of > "Add Engines..." it could be "Manage Engines..." which would pop up the same > part of the Options GUI you described. > > Also, engines should generally be stored in the profile, but still allowing > search plugins in the browser root allows for cross-profile search plugins. It > would make sense to keep the default Google search in the browser root and let > users add new searches to the profile. It would be nice if regular plugins were > handled this way too, but that's another bug. I totally agree there should be engine management. There search bar should be able to have subfolders (like Bookmarks), several arrangement methods, and right-click Properties (including 'Name') access.

Hmm, I think the last proposal is perhaps going a little far. I don't see the need for subfolders, or any method of sorting than by name, for a default Firefox install (although the ability to remove search engines is important). I think perhaps some of these would be better provided by extensions, in the interests of avoiding UI and code bloat.

(In reply to comment #20) > Hmm, I think the last proposal is perhaps going a little far. I don't see the > need for subfolders, or any method of sorting than by name, for a default Firefox > install (although the ability to remove search engines is important). I think > perhaps some of these would be better provided by extensions, in the interests > of avoiding UI and code bloat. I agree about folders, but something must be there for the user to be able to sort the engines the way that pleases him. He may prefer to have most important ones in the first entries of the list and unused ones far away.

(In reply to comment #21) > I agree about folders, but something must be there for the user to be able to > sort the engines the way that pleases him. He may prefer to have most important > ones in the first entries of the list and unused ones far away. IMO, it should be a simple extension of what there is now: - add right-click menu with a delete and rename option - allow the user to drag-and-drop the engines within the drop-down, the same way that you can drag-and-drop bookmarks. This will enable re-ordering of the engines All I really care about is right-click to delete.

Comment 22 seems the most sensible option - any other functionality should be the realm of extenstions (at least for now).

Any possibility of having this for FF 1.1?

It would be convenient if the search bar and "Quick searches" (keywords in bookmarks) could be merged. This would negate the need for search engine management, as the engines would simply be bookmarks. One idea would be to have several other options (like "keyword") in a bookmark including: 1) "Use as search engine" on/off - Becomes one of the items in the search bar 2) "Use in <selected text> context menu" on/off - Shows up when right clicking on selected text 3) "Use in page/link context menu" on/off - shows up when right clicking the page or a link and uses the respective URL for the search. This would allow things like 'show google cache of page' to be added as a context menu item. As additional benefit (at least for me) would be that I wouldn't need a lot of separate extensions I use now (like context menu dictionary lookup) and adding the functionality to another computer would simply involve moving my bookmarks, not installing a bunch of extensions a second/third/fourth... time. Just my 2 cents

(In reply to comment #24) > Any possibility of having this for FF 1.1? Yes I'd think so as it may not be too difficult to implement, depending on what we decide here and it would seem logical, given as 1.1 is supposed to be for UI changes and other more 'minor' features.

I'd rather have it merged with the find bar instead of bookmarks: with a bit of UI that would allow to easily search in the result page for the same search terms that were submitted to the web site.

*** Bug 271207 has been marked as a duplicate of this bug. ***

> I totally agree there should be engine management. There search bar should be > able to have subfolders (like Bookmarks), several arrangement methods, and > right-click Properties (including 'Name') access. With due respect to comments #20 and #21, I think the search bar should be able to have folders. I currently have more than 30 search plugins for my search bar -- I may be unusual in that regard (I'm a journalist for a living), but still, it would be nice to be able to categorize them properly. Better to have the capability and not need it than to need it and not have it.

Folders. The only thing I can think of to better Firefox is folders in the search bar. After that comes organization, which is also good.

*** Bug 275209 has been marked as a duplicate of this bug. ***

*** Bug 275327 has been marked as a duplicate of this bug. ***

I meant to submit this as a bug over a year ago and forgot. I just submitted it as bug 275327 (after extensive searching honestly) then found it had already been submitted. THe lack of deletion part of this bug is a major, obvious, easily fixable bug that affects anyone who uses search. A proper SE management GUI would be nice, but that is more of a long-term priority. Can we just have make sure that for 1.1 the basic functionality is included: pressing [shift]+[del] while an engine on the drop-down list is selected should result in that engine being deleted (in the same way that a searched for phrase can be deleted). Bug 275327 ("RFE: ability to delete search engines) could be used for the simple delete thing if this bug is too cluttered. (Also, possibly allow the use of the [del] key on its own as many users won't think of using [shift]+[del]. I think, if we do that, there has to be a confirmation dialog box though (as it is too easy to hit a single key) and I would still allow [shift]+[del] to quickly delete.) This is very important because (as I noted in bug 266227) many SEs are now included by default which many people wouldn't ever use (e.g.: the top two are exactly the same SE and two of them are for shopping sites) and it is very difficult for users currently to replace them with SEs that are more appropriate to themselves. (BTW, is their a bug to discuss which SEs should be included by default?)

I agree with Comment #33, but maybe a right-click menu would be neater (and more visible) than [shift]+[del]? A keyboard shortcut could also be included for accessibility.

Quoting Comment #34: >>maybe a right-click menu would be neater (and more visible) than [shift]+[del]<< I did not suggest this because, apparently, (secondary-click) context menus coming off drop-down menus is considered to be against people's expectations of WIMP GUIs. I happen to disagree but I asked for this on the back/forward buttons and it was WONTFIXed. For one thing the menu bar at the top of windows in most WIMP programs can have context menus coming off it. Why are other drop-down menus not allowed this when the most commonly used drop-down menu is? If the developers no longer have a hang-up about this, a context menu with "disable", "remove", "move up" and such like would be a good way of dealing with many of the SE management problems. A full dialog-box-based UI would still be nice but would eventually supplement this basic behaviour and would be needed for, say, adding or enabling engines without downloading them from a webpage or easier re-organisation. This would be similar to the bookmarks context menu and manager. In fact the SE toolbar could just be a folder within ones bookmarks to which one could manually add SEs using bookmarklets or replacement strings. The SE management functionality could just be added to bookmark manager. (BTW, is there a bring back the find "SE" to the toolbar RFE as this *is* useful when one is searching for something on a page? And/or, we could maybe have an easy way to copy the phrase from the SE toolbar to the find toolbar.)

(In reply to comment #35) > I did not suggest this because, apparently, (secondary-click) context menus > coming off drop-down menus is considered to be against people's expectations of > WIMP GUIs. > > I happen to disagree but I asked for this on the back/forward buttons and it was > WONTFIXed. For one thing the menu bar at the top of windows in most WIMP > programs can have context menus coming off it. Why are other drop-down menus > not allowed this when the most commonly used drop-down menu is? This is _very_ inconsistent - the Bookmarks menu in Firefox has a context menu, therefore why can't the Search Engines drop down?

>>This is _very_ inconsistent<< What is very inconsistent? I'm arguing for comment #34 .

*** Bug 285114 has been marked as a duplicate of this bug. ***

Sorry for the (maybe) stupid question, but what does "Status Whiteboard [asaP1]" mean?

*** Bug 293999 has been marked as a duplicate of this bug. ***

Duped bug 293999 describes dataloss problem with upgrade from 1.0.3 to 1.0.4 (user's selection of search modules was lost after upgrade), so I'm adding keyword and raise severity.

Another possible way of managing search engine extensions in the toolbar is by implementing a 'right-click' menu, similar to the 'right-click' menu used for bookmarks.

This seems more like a "BUG OF OMISSION" than of commission. There isn't any means of management as pointed out.So amoung many things that may eventually be included I'd like to add a few more to the wish list: 1.) A "SORT" function (with options for Global Sort or by Group/Cataglory or Seperator. 2.) SEPERATORS as also currently provided in "Bookmarks" for the browser. Do you think people who don't see well would like COLORED SEPERATORS? In short, Management with the same features as in Bookmarks.

FWIW, there's currently an extension, that adds a context menu to the drop-down list, with an option to delete the search plugin. Maybe the author can be contacted about checking the code into Firefox. http://www.extensionsmirror.nl/index.php?showtopic=3614 Also, maybe this bug should be split into 'sub-bugs' - delete search plugin - re-order list - separators If this bug is just about remove a search plugin, the summary should be changed. I don't see much point in reordering the list, "manage engines", etc. A delete function is all that is needed.

(In reply to comment #44) > I don't see much point in reordering the list, "manage engines", etc. A delete > function is all that is needed. You find reinstalling all your search engines each time you upgrade fun? On Mac, at least, I have to manually extract the searchplugins folder, upgrade, move the folder to the new copy of Firefox, and hope the copy takes. I have nearly 20 search plugins. I shouldn't have to reinstall them every time -- my bookmarks transfer, my extensions transfer, my passwords transfer, and my cookies transfer. Why can't my search engines transfer, too? It is astounding that this issue still remains unfixed, especially since the report is now over a full year old and was reported while the browser was still called Firebird and we're now at 1.0.6 and all other major features migrate with no trouble.

(In reply to comment #45) > You find reinstalling all your search engines each time you upgrade fun? On Mac, > at least, I have to manually extract the searchplugins folder, upgrade, move the > folder to the new copy of Firefox, and hope the copy takes. Using the trunk, search plugins are kept in the profile folder (on Windows here) as per bug 123315 . I don't know about Mac. But this is what I mean about splitting this up into sub-bugs.

Maybe you want to have a look at the extension "Search Engine Ordering". It has a UI to rearrange and delete the search engines. http://maltekraus.ma.funpic.de/Firefox/Search-Engine-Ordering/index.php?lang=en Maybe it could be ported into the code of Firefox.

I've managed to organize/sort my search plugins by putting them in subdirectories in the "C:\Program Files\Mozilla Firefox\searchplugins" folder. This may be useful for a lot of people as a temporary solution without using the SEO plugin: I wanted certain search engines on top(google, wikipedia, dictionary.com, php doc, mysql search, imdb, etc.), and certain more seldomly used engines at the bottom(ebay, pricewatch, hyperdictionary, ticketmaster, etc.), so I tried using the windows file system to sort the plugins alphabetically, by date, etc. in the default 'searchplugins' directory. This seems to work after you restart Firefox, so I wondered, couldn't I just create subdirectories in "C:\Program Files\Mozilla Firefox\searchplugins" and sort them this way? Turns out, Firefox does look for search plugins in subdirectories in the order that the file system has been configured to list them. So I just created different tiered categories for different sets of plugins and put them in alpha-numeratized directories like so: C:\Program Files\Mozilla Firefox\searchplugins\1\1 - Google\ C:\Program Files\Mozilla Firefox\searchplugins\1\2 - Wikipedia\ C:\Program Files\Mozilla Firefox\searchplugins\1\3 - Dictionary.com\ C:\Program Files\Mozilla Firefox\searchplugins\1\4 - Thesaurus.com\ C:\Program Files\Mozilla Firefox\searchplugins\1\5 - Everything2\ C:\Program Files\Mozilla Firefox\searchplugins\2\1 - Answers.com\ C:\Program Files\Mozilla Firefox\searchplugins\2\2 - IMDB\ C:\Program Files\Mozilla Firefox\searchplugins\2\3 - CDDB\ C:\Program Files\Mozilla Firefox\searchplugins\2\4 - PHP\ C:\Program Files\Mozilla Firefox\searchplugins\2\5 - MySQL\ etc. However, I've recently noticed that when Firefox updates the search plugins I've installed, it just automatically installs them to 'C:\Program Files\Mozilla Firefox\searchplugins\*', thus creating duplicate entries of the search plugins I've already organized. This is very frustrating so I hope that in future versions Firefox can just replace the existing plugins in the subdirectory they're installed to. Lastly, the SEO plugin seems like a very useful tool, but it doesn't come standard with Firefox, and I'm sure a lot of people haven't heard about it. Maybe something like it could be built into future versions of Firefox as this is a desperately needed feature, and the subdirectories approach doesn't seem to work well with Firefox's auto-update component.

*** Bug 253891 has been marked as a duplicate of this bug. ***

*** Bug 255543 has been marked as a duplicate of this bug. ***

Note that I just tried Firefox 1.5 beta1 on Linux, and the Del key doesn't work to remove searchplugins (I restarted browser too). Was this functionality removed or it's a bug?

Marius: When this bug is fixed it will be marked FIXED, until then don't expect this functionality in Firefox.

From my suggestion in related bug 205011c#89: I think the search engines and options should be selectable via UI (like Thunderbird's newsgroup "Subscribe" UI), and not force the user to visit some (confusing) website. It would all be *right there* in "context". +---------------+ |[G] mozilla | +---------------+ | Google | | Yahoo | | ... | |---------------| | Options... | <--- Click this, to get "Search Bar Options" +---------------+ +- Search Bar Options -------------------------------+ | _//Add Search Engines\\_/Change Order\_/Settings\_ | ||+-------------------------+ || ||| A9 [ ] /\| [ Add ] || ||| Acronym Finder [x] ||| [ Remove ] || ||| Google DE [ ] ||| [ Refresh List ] || ||| Leo Eng<->Ger [x] ||| [ Stop ] || ||| Thesaurus [ ] \/| || ||+-------------------------+ _Search Homepage_ || |+--------------------------------------------------+| | [[ OK ]] [Cancel] | +----------------------------------------------------+ +- Search Bar Options -------------------------------+ | _/Add Search Engines\_//Change Order\\_/Settings\_ | ||+-------------------------+ || ||| A9 /\| [ Move Up ] || ||| Acronym Finder ||| [ Move Down ] || ||| Google DE ||| [ Alphabetical] || ||| Leo Eng<->Ger ||| || ||| Thesaurus \/| || ||+-------------------------+ || |+--------------------------------------------------+| | [[ OK ]] [Cancel] | +----------------------------------------------------+ +- Search Bar Options -------------------------------+ | _/Add Search Engines\_/Change Order\_//Settings\\_ | || || || Open search reslts in: || || (o) Currently open tab/window || <-- Default!? || ( ) New tab || || ( ) New window || || || || Size of Search Bar: [ ] [pixels\/] [Default] || <-- pixels, %, em ? || || |+--------------------------------------------------+| | [[ OK ]] [Cancel] | +----------------------------------------------------+ What do you think?

I think it can all be done in one window. Base it on the first one, except remove 'Refresh list' - the UI should do refresh itself right away, this is just UI clutter. Also remove 'Stop' - what on earth should a user expect that button to do? It's completely unescessary. From the second page - we need 'move up' and 'move down', but we don't need/want alphabetical ordering. From the third page - the opening in new window/tab etc options are nice but probably UI clutter. If we want those, we could have an 'advanced' button that changes them. I'm not convinced that they are needed though. We do want the size-changing (more on this next). With the size-changing, there's no way we'd want users to enter a size in pixels, how is your mom going to understand that? There's two things I can think of.. either an option list with small, medium, large, extra-large, or a slider to change the size. We could potentially have the slider use the same small, medium etc terminology. Anyway, there's no way that you'll get the UI guys to approve that three-tabbed suggestion you've got there. That's half as many tabs as the entire Firefox preferences section. One window, or POSSIBLY a second advanced tab (and only if we really, really see the need for it, which I doubt) would be accepted. GUI clutter is not acceptable in Firefox.

(In reply to comment #54) > remove 'Refresh list' - the UI should do refresh itself right away There are 1000+ search plugins. Immediate refreshing would be a nightmare, at both ends (client & server). > Also remove 'Stop' - what on earth should a user expect that button to do? See above. Do you understand the way Thunderbird's newsgroup & IMAP *Subscriptions* are for? There's a reason; and it's the same reason as here. > we don't need/want alphabetical ordering. I think it would be useful to *quickly* undo a mess. (Agreed: optional) BTW: Who is "we"? Are you an official representative? Are you "royal"? ;-) > the opening in new window/tab etc options are nice but probably UI clutter. I've been annoyed by my current page being overwritten. I (we) would like it. > 'advanced' button that changes them. How do you suggest to incorporate both reordering *and* options into one tab? > With the size-changing, there's no way we'd want users to enter a size in > pixels, how is your mom going to understand that? She would never even think to change the size. :-P Users would see the default setting and easily deduct a proportional increase/decrease amount (hence the "Default" button to get out of trouble). It requires no knowledge, only cognition. > an option list with small, medium, large, extra-large, or a slider "We" think that would be OK too. > That's half as many tabs as the entire Firefox preferences section. ...and 316% more than in the File menu. So what? The user should see (only) those options that he needs, in *context*. UI clutter is, IMO, when users see more options than they need within the context they are in. Here, they are only in the search bar context, and per default see only the most-likely used and non-cluttered add/remove search engines tab. The criteria should be "clutter in context", not the sum of all UI, visible or not. All the user would ever see is the one menuitem "Options" in the search bar dropdown. That's no more than he sees now (and wouldn't go to a truly cluttered webpage). > GUI clutter is not acceptable in Firefox. Obviously. The real question is: What is clutter? Is *this* clutter?

(In reply to comment #55) > Obviously. The real question is: What is clutter? Is *this* clutter? Imagine you are Joe User who is confronted with this UI. I'm sure it would scare you. We (the users) wouldn't want that, would we (the users) ?

Since there are so many search engines (1,000+), there would need to be a "Search" feature (like Thunderbird's "Search Bar"): +- Search Bar Options -------------------------------+ | _//Add Search Engines\\_/Change Order\_/Settings\_ | || Search for: [ ] || <-- add search/filter box ||+-------------------------+ || ||| A9 [ ] /\| [ Add ] || ||| Acronym Finder [x] ||| [ Remove ] || ||| Google DE [ ] ||| [ Refresh List ] || ||| Leo Eng<->Ger [x] ||| [ Stop ] || ||| Thesaurus [ ] \/| || ||+-------------------------+ _Search Homepage_ || |+--------------------------------------------------+| | [[ OK ]] [Cancel] | +----------------------------------------------------+ So entering "trans" would show only entries that contained "trans" (like tranlate de-en, tranlate de-fr). The "Refresh List" and "Stop" button could be eliminated, if the list refreshes itself every time the user selects: Search bar menu / Options. But I fear that might be annoying if users go into there repeatedly, as there would always be a long delay with no/reduced UI response while it's updating. Also, the aforementioned server hits.

(In reply to comment #55) > (In reply to comment #54) > > the opening in new window/tab etc options are nice but probably UI clutter. > > I've been annoyed by my current page being overwritten. I (we) would like it. > Just Alt + Enter instead of Enter. The same applies to location bar.

BTW: (to answer your question) I find the suggested UI *much* better than anything suggested so far (which is nothing), and *much* better than the current method (via truly cluttered webpage). Oh, and completely non-scary. What's scary about an existing list of items that you put checkmarks next to? It's exactly what the user wnted.

> There are 1000+ search plugins. Immediate refreshing would be a nightmare, at both ends (client & server). Oh so this is for updating them from the server rather than to make the changes to list you see refreshed. Well, ok add 'confusing' and 'badly' named to my list of complaints about it. Plus we shouldn't have to worry about updating search engines manually really, and if we do, the update checking UI should be integrated with the UI for all the other update stuff (possibly openable from the search engine UI however). >> we don't need/want alphabetical ordering. >I think it would be useful to *quickly* undo a mess. (Agreed: optional) Anything 'optional' isn't going to make it past the GUI designers. I'd be happy with a hidden pref. >BTW: Who is "we"? Are you an official representative? Are you "royal"? ;-) No, but I'm from the UK, perhaps I speak the Queen's English? ;) >> the opening in new window/tab etc options are nice but probably UI clutter. >I've been annoyed by my current page being overwritten. I (we) would like it. I've been annoyed by this too, but I'm 50/50 as to whether it's worthwhile to have a UI for it. Don't forget that alt+enter opens it in a new tab. Perhaps we could by default synch this to the options in Options>Tabs >> 'advanced' button that changes them. >How do you suggest to incorporate both reordering *and* options into one tab? Reordering is easy, just move up/down buttons which are quite standard. Advanced options could be another tab or a button, but as you can tell I wouldn't be very happy with either, as they're both fairly ugly. >> With the size-changing, there's no way we'd want users to enter a size in >> pixels, how is your mom going to understand that? >She would never even think to change the size. :-P Users would see the default >setting and easily deduct a proportional increase/decrease amount (hence the >"Default" button to get out of trouble). It requires no knowledge, only >cognition. Personally, I don't believe the resizing belongs here anyway, I think (as I've mentioned in bug 205011) there should simply be a resizer added automatically between resizable entries (like location bar and find bar). >> That's half as many tabs as the entire Firefox preferences section. >...and 316% more than in the File menu. So what? The user should see (only) >those options that he needs, in *context*. UI clutter is, IMO, when users see >more options than they need within the context they are in. Here, they are only >in the search bar context, and per default see only the most-likely used and >non-cluttered add/remove search engines tab. The criteria should be "clutter in >context", not the sum of all UI, visible or not. All the user would ever see is >the one menuitem "Options" in the search bar dropdown. That's no more than he >sees now (and wouldn't go to a truly cluttered webpage). >> GUI clutter is not acceptable in Firefox. >Obviously. The real question is: What is clutter? Is *this* clutter? I'm afraid it is, and rather bad clutter at that. To me, it looks like a design by comittee, of trying to put everything in there that a user could possibly want to do. That UI design belongs in Seamonkey, but there is 0% chance that you'll get it past the people who have the final say on the UI in Firefox. Someone should change severity to 'enhancement', version to 'trunk' and target milestone to 'future' (though I feel this is a must-have for Firefox 2). Also I'm dubious that bug 293999 is a dup of this bug, surely it's a dup of bug 123315 instead, in which case we should remove the dataloss keyword from this bug also.

How possible/impossible would it be to simply use the Extension Manager window for this (calling it Search Engine manager offcourse) ? The devs agreed that the current Opions UI sux and has to be changed (again), so adding something similar , as your current proposal, will never happen. (ps: talk to Mike Beltzner if you want to know more about it.)

I liked the idea of Peter very much. However, I also think 3 tabs is too much. How about this window? (Also reachable via "Options..." below search plugin list as a dropdown from search field). +- Search Bar Options -------------------------------------+ | | | +-Plugin Repository-----+ +- Installed plugins ---+ | | | + Translations /\| | Google /\| | | | - Dictionaries ||| | Dictionaries ||| | | | + Britannica ||| | Britannica ||| | | | + Merriam-webster ||| | Merriam-webster ||| | | | + ABC ||| | ABC ||| | | | ... \/| | \/| | | +-----------------------+ +-----------------------+ | | [ Install => ] [Remove] [Up] [Down] [Sort]| | | | [Advanced] | +----------------------------------------------------------+ | [[ OK ]] [Cancel] | +----------------------------------------------------------+ On the left: Tree selection with hierarchical view of all serach plugins (categories as already on the site). You would only need to refresh the category that the user expands (delay too long? => prefetch of 1 level?). On the right: Regular list of installed plugins. Both lists could be multi-select lists to install/delete more than one plugin at once. Up/down makes no sense and should be greyed out if more than one entries are selected. all buttons should be disabled if nothing is selected. Consider [sort] and [advanced] as optional. Opinions?

In believe that even the "reduced" variant would be quite complicated. Using those add/remove, up/down buttons is so damn annoying for the advanced users. So, to solve this it should be possible to use drag & drop reordering, and that could be done as well in the select dialog directly (http://maltekraus.de/Firefox/Search-Engine-Ordering/drag-drop.png). I wouldn't try to add all possible search engines to the available ones, I'd say: Just keep the ones that are currently included by default anyway, and allow adding them similar to "Add a Keyword for this Search". (Maybe have some hand selected ones in a popup dialog on "add engines" and explain how adding not listed ones works). This would be much more intuitive than searching a list of about 2000 plugins. As advanced user I rather tend to find out a search query myself and create it manually rather than searching that endless list. Like "inline drag & drop" reordering removing could be done via hovering an item and pressing the DEL key. It isn't obvious, but better than adding a bloat dialog.

(In reply to comment #61) > How possible/impossible would it be to simply use the Extension Manager window > for this (calling it Search Engine manager offcourse) ? Makes sense. That way, the same existing code can be reused again and again. Of course, it would be nice if THAT code allowed sorting by column, for example. ;-)

These are now stored the in the profile. Apparently Ben has some search bits he's already working on.

*** Bug 318010 has been marked as a duplicate of this bug. ***

*** Bug 325673 has been marked as a duplicate of this bug. ***

This is the base work for the new engine manager dialog. It's not 100% complete, but I want some initial feedback on it before I continue. Supports reordering, removing, editing of aliases. Some known issues: - the alias is unused - setting it has no effect - extension installed search plugins need to be handled better (disabling and then reenabling an extension that provides a removed plugin causes the plugin to reappear) - drag and drop should be implemented in the treeview - removing all engines leads to non-optimal UI behavior - missing icon padding in the tree view And undoubtedly other bugs too. I'd love it if people could give this patch a spin and see if they can shake any bugs out of it. I will try to post linux and windows test builds tomorrow that include this patch, for people who want to test it out.

I've posted test Windows and Linux builds that include this patch at http://gavinsharp.com/ff/ .

Just tested it and it looks nice. I like the context menu integration. - I assume the alias will be used as display name. If not, a rename is missing. - Found one problem. Reordering doesn't always work when more then one window is opened. - Will there be seperators and/or sub menus? - Will the results be opened in a new tab when started from the search bar, just like the context menu search?

(In reply to comment #70) > - I assume the alias will be used as display name. If not, a rename is missing. No, the alias is meant to be used as a "keyword" in the location bar, much like bookmark keywords today. The unique identifier for a search engine is currently it's display name, so having a "rename" is not trivial. This can (and hopefully will) change in the future, but won't be part of the initial implementation. > - Found one problem. Reordering doesn't always work when more then one window > is opened. I did test this case - can you consistently reproduce the issue? Some steps to reproduce would be useful, but I'll look into it and see if I see anything. > - Will there be seperators and/or sub menus? Possibly, but I don't see this as something that would be useful to most people. > - Will the results be opened in a new tab when started from the search bar, > just like the context menu search? There is a pref for always openeing the results in a new tab (browser.search.openintab), or you can use Alt+Enter to open a specific search in a new tab. This patch doesn't affect this behavior.

Are there plans to add drag & drop just as with the bookmarks? I find it much more useful to move an engine just from the menu than having to open a dialog. When using (and writing) the extension 'Search Engine Ordering' I noticed that I used it much more often after I implemented Drag 'n' Drop. Same thing for a context menu to remove an engine like in the extension 'SearchPluginHacks' and 'Search Engine Ordering'. Another minor thing: when the dialog is closed, the menu reappears and is hidden shortly after that. Maybe a hidePopup() could be called before the dialog is opened to prevent that flickering?

(In reply to comment #72) > Are there plans to add drag & drop just as with the bookmarks? I find it much > more useful to move an engine just from the menu than having to open a dialog. > When using (and writing) the extension 'Search Engine Ordering' I noticed that > I used it much more often after I implemented Drag 'n' Drop. Yes, see comment 68. > Another minor thing: when the dialog is closed, the menu reappears and is > hidden shortly after that. Maybe a hidePopup() could be called before the > dialog is opened to prevent that flickering? That's an issue with the modal dialog - a workaround for that is indeed another bug that needs fixing.

(In reply to comment #71) > (In reply to comment #70) > > - Found one problem. Reordering doesn't always work when more then one window > > is opened. > > I did test this case - can you consistently reproduce the issue? Some steps to > reproduce would be useful, but I'll look into it and see if I see anything. Strange, when I first time opened your version it went wrong quite often, but once I restarted "Bon Echo" it didn't happen anymore. > There is a pref for always openeing the results in a new tab > (browser.search.openintab), or you can use Alt+Enter to open a specific search > in a new tab. This patch doesn't affect this behavior. Well, one would at least expect them to behave similar: both in same window of both in new tab. Anyway, found that it is bug 227710.

After adding a new search engine I had a strange refresh problem. I will attach a screendump. The top window is opened by the underlining manage dailog. I added a search engine from mycroft and after that the dropdown list from the underlying window opened and didn't go away until I clicked on it.

(In reply to comment #76) > Created an attachment (id=219141) [edit] > Refresh problem after adding, see comment 75 Thanks Frank, I'd noticed that too, and I've fixed this locally. It's a pretty minor change, so I'll wait until there are more changes before posting an updated patch and new test builds.

Comment on attachment 219110 [details] [diff] [review] search engine manager, v1 var foundStart = false; var startLine, numberOfLines; // Find the beginning and end of the section - for (var i=0; i<lines.length; i++) { + for (var i = 0; i<lines.length; i++) { I don't get why you fixed spacing around one operator, but not the other :P var section = {}; - for (var i=0; i<lines.length; i++) { + for (var i = 0; i<lines.length; i++) { same as previous comment! // NO NON-EMPTY & <name>=<value> TEMPLATE?<n1>=<v1>&<n2>=<v2> - for (var i=0; i<inputs.length; i++) { + for (var i = 0; i<inputs.length; i++) { and again? (just fix all of these, and I'll stop yelling at you) + // Now that all engines are loaded, build the sorted engine list + this._buildSortedEngineList(); + + // Remove prefs for non-existent engines + this._clearOldPrefs(); Can this happen after we select the engine? or even async to startup? I don't know how expensive this is or if its worth it, but doing cleanup that doesn't affect functionality shouldn't happen as part of startup + _saveSortedEngineList: function SRCH_SVC_saveSortedEngineList() { <snip> + // Save the pref file explicitly, since we're called at XPCOM shutdown + preferences.savePrefFile(null); + }, discussed this on IRC, prefs are an implementation detail right now, so this is ok for a2, but need to clean this up when we switch away from prefs + _buildSortedEngineList: function SRCH_SVC_buildSortedEngineList() { + // Array for the remaining engines, alphabetically sorted the alpha engines thing shouldn't really happen in most cases, I'd think that we would add things to the sort order when they're being added, or detected in loadEngines. If we want to allow alpha sorting, expose that in the management interface notifyAction(engineToRemove, SEARCH_ENGINE_REMOVED); } }, + moveEngine: function SRCH_SVC_moveEngine(aEngine, aDir) { + ENSURE_ARG(Math.abs(aDir) < this._sortedEngines.length, + "Invalid offset passed to moveEngine!"); + ENSURE_ARG(aEngine instanceof Ci.nsISearchEngine, + "Invalid engine passed to moveEngine!"); + + var engine = aEngine.wrappedJSObject; + + var index = this._sortedEngines.indexOf(engine); + ENSURE(index != -1, "moveEngine: Can't find engine to move!", + Cr.NS_ERROR_UNEXPECTED); + + var newIndex = index - aDir; + ENSURE_ARG((newIndex < this._sortedEngines.length) && (newIndex >= 0), + "Index out of bounds!"); + + // Swap the two engines + this._sortedEngines[index] = this._sortedEngines[newIndex]; + this._sortedEngines[newIndex] = engine; + + notifyAction(engine, SEARCH_ENGINE_CHANGED); + }, Why does this only move one step at a time? can we not use splice() here instead? get defaultEngine() { const defPref = BROWSER_SEARCH_PREF + "defaultenginename"; // Get the default engine - this pref should always exist, but the engine // might be hidden this._defaultEngine = this.getEngineByName(getLocalizedPref(defPref, "")); if (!this._defaultEngine || this._defaultEngine.hidden) - this._defaultEngine = this.getVisibleEngines({})[0] || null; + this._defaultEngine = this._getSortedEngines(false)[0] || null; why change this? net result is the same? what's the aCount useful for in getVisibleEngines anyway? @@ -2029,16 +2139,17 @@ SearchService.prototype = { if (aVerb == SEARCH_ENGINE_LOADED) { var engine = aEngine.QueryInterface(Ci.nsISearchEngine); LOG("nsISearchEngine::observe: Done installation of " + engine.name + "."); this._addEngineToStore(engine.wrappedJSObject); } break; case XPCOM_SHUTDOWN_TOPIC: + this._saveSortedEngineList(); I'd rather save stuff earlier than XPCOM shutdown, can't we do this on quit-application-requested or something a little earlier, aside from the "drop prefs as impl detail" bit. + var os = Cc["@mozilla.org/observer-service;1"]. + getService(Ci.nsIObserverService); + os.addObserver(this, "browser-search-engine-modified", + false); why the linebreak for this? + /** + * Moves the selected engine either up or down in the engine list + * @param aDir + * -1 to move the selected engine down, +1 to move it up. + */ + move: function engineManager_move(aDir) { + var selectedEngine = gEngineView.selectedEngine; + if (!selectedEngine) + return; + + if (Math.abs(aDir) != 1) + throw new Error("Invalid direction!"); + + var newIndex = gEngineView.selectedIndex - aDir; + + if (newIndex < 0 || newIndex > gEngineView.lastIndex) + throw new Error("Invalid aDir!"); + + gEngineView._engineStore.moveEngine(selectedEngine, aDir); + + gEngineView.invalidate(); + gEngineView.selection.select(newIndex); + }, + + onSelect: function engineManager_onSelect() { + var noEngineSelected = (gEngineView.selectedIndex == -1); + document.getElementById("remove").disabled = noEngineSelected; + // document.getElementById("rename").disabled = noEngineSelected; + document.getElementById("alias").disabled = noEngineSelected; + + document.getElementById("up").disabled = noEngineSelected || + (gEngineView.selectedIndex == 0); + + var lastSelected = (gEngineView.selectedIndex == gEngineView.lastIndex); + document.getElementById("dn").disabled = noEngineSelected || + lastSelected; + } +}; I think this would be nicer if it supported move to top/move to bottom and dnd (i.e. more than one step at a time) Index: browser/components/search/content/engineManager.xul =================================================================== RCS file: browser/components/search/content/engineManager.xul diff -N browser/components/search/content/engineManager.xul --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ browser/components/search/content/engineManager.xul 20 Apr 2006 05:21:43 -0000 @@ -0,0 +1,60 @@ +<?xml version="1.0"?> no boilerplate! +<?xml-stylesheet href="chrome://global/skin/"?> + +<!DOCTYPE dialog SYSTEM "chrome://browser/locale/engineManager.dtd"> + +<dialog id="engineManager" + xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" + buttons="accept,cancel" + onload="gEngineManagerDialog.init();" + ondialogaccept="gEngineManagerDialog.onOK();" + title="&engineManager.title;" + style="min-width: 40em;" + windowtype="Browser:SearchManager"> localize the minwidth! + <label>&engineManager.intro;</label> <description> please, so this wraps nicely if and when the string is more verbose + <hbox flex="1"> + <tree id="engineList" flex="1" rows="10" hidecolumnpicker="true" + seltype="single" onselect="gEngineManagerDialog.onSelect();"> + <treechildren id="engineChildren" flex="1"/> + <treecols> + <treecol id="engineName" class="sortDirectionIndicator" flex="3" + persist="width" label="&nameColumn.label;"/> + <splitter class="tree-splitter"/> + <treecol id="engineAlias" class="sortDirectionIndicator" flex="2" + persist="width" label="&aliasColumn.label;"/> + </treecols> + </tree> + <vbox> + <spacer flex="1"/> + <button id="up" label="&up.label;" accesskey="&up.accesskey;" + oncommand="gEngineManagerDialog.move(1);"/> + <button id="dn" label="&dn.label;" accesskey="&dn.accesskey;" + oncommand="gEngineManagerDialog.move(-1);"/> + <spacer flex="1"/> + <button id="alias" label="&editAlias.label;" + accesskey="&editAlias.accesskey;" + oncommand="gEngineManagerDialog.editAlias();"/> + <button id="remove" label="&delete.label;" + accesskey="&delete.accesskey;" + oncommand="gEngineManagerDialog.remove();"/> These should be commands instead of oncommand, so we can skip sanity checks in the commands in favour of disabling the command when appropriate (also, better/expected UI to do this) Not sure about the layout, will play with it next iteration Index: browser/locales/en-US/chrome/browser/searchbar.dtd =================================================================== RCS file: browser/locales/en-US/chrome/browser/searchbar.dtdpff diff -N browser/locales/en-US/chrome/browser/searchbar.dtd --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ browser/locales/en-US/chrome/browser/searchbar.dtd 20 Apr 2006 05:21:45 -0000 @@ -0,0 +1,2 @@ +<!ENTITY cmd_engineManager.label "Manage search engines..."> +<!ENTITY cmd_engineManager.accesskey "M"> Should be Manage Search Engines not there yet, but not that far off

> (In reply to comment #70) > > - I assume the alias will be used as display name. If not, a rename is missing. > > No, the alias is meant to be used as a "keyword" in the location bar, much like > bookmark keywords today. The unique identifier for a search engine is currently > it's display name, so having a "rename" is not trivial. This can (and hopefully > will) change in the future, but won't be part of the initial implementation. Filed bug 335102 for that.

(In reply to comment #78) > (From update of attachment 219110 [details] [diff] [review] [edit]) > - for (var i=0; i<inputs.length; i++) { > + for (var i = 0; i<inputs.length; i++) { > > and again? (just fix all of these, and I'll stop yelling at you) fixed all of these > + // Now that all engines are loaded, build the sorted engine list > + this._buildSortedEngineList(); > + > + // Remove prefs for non-existent engines > + this._clearOldPrefs(); > > Can this happen after we select the engine? or even async to startup? I don't > know how expensive this is or if its worth it, but doing cleanup that doesn't > affect functionality shouldn't happen as part of startup Actually, I think I'll just remove that method. Having extra prefs around for now won't really hurt, and clearing them causes problems with temporarily disabled extensions that ship search engines. It won't be relevant when we move to non-prefs for storage, which I want to do as soon as this is landed. > + _saveSortedEngineList: function SRCH_SVC_saveSortedEngineList() { > <snip> > + // Save the pref file explicitly, since we're called at XPCOM shutdown > + preferences.savePrefFile(null); > + }, > > discussed this on IRC, prefs are an implementation detail right now, so this is > ok for a2, but need to clean this up when we switch away from prefs > > + _buildSortedEngineList: function SRCH_SVC_buildSortedEngineList() { > > + // Array for the remaining engines, alphabetically sorted > > the alpha engines thing shouldn't really happen in most cases, I'd think that > we would add things to the sort order when they're being added, or detected in > loadEngines. If we want to allow alpha sorting, expose that in the management > interface This code just maintains the current behavior for unordered engines. Added engines are added to the end of the list. Do you want me to remove this? Guess it doesn't really matter. > + moveEngine: function SRCH_SVC_moveEngine(aEngine, aDir) { > + ENSURE_ARG(Math.abs(aDir) < this._sortedEngines.length, > + "Invalid offset passed to moveEngine!"); > + ENSURE_ARG(aEngine instanceof Ci.nsISearchEngine, > + "Invalid engine passed to moveEngine!"); > + > + var engine = aEngine.wrappedJSObject; > + > + var index = this._sortedEngines.indexOf(engine); > + ENSURE(index != -1, "moveEngine: Can't find engine to move!", > + Cr.NS_ERROR_UNEXPECTED); > + > + var newIndex = index - aDir; > + ENSURE_ARG((newIndex < this._sortedEngines.length) && (newIndex >= 0), > + "Index out of bounds!"); > + > + // Swap the two engines > + this._sortedEngines[index] = this._sortedEngines[newIndex]; > + this._sortedEngines[newIndex] = engine; > + > + notifyAction(engine, SEARCH_ENGINE_CHANGED); > + }, > > Why does this only move one step at a time? can we not use splice() here > instead? This method isn't limited to one at a time, I just forgot to update the interface docs. > get defaultEngine() { > const defPref = BROWSER_SEARCH_PREF + "defaultenginename"; > // Get the default engine - this pref should always exist, but the engine > // might be hidden > this._defaultEngine = this.getEngineByName(getLocalizedPref(defPref, "")); > if (!this._defaultEngine || this._defaultEngine.hidden) > - this._defaultEngine = this.getVisibleEngines({})[0] || null; > + this._defaultEngine = this._getSortedEngines(false)[0] || null; > > why change this? net result is the same? what's the aCount useful for in > getVisibleEngines anyway? Just to make it clearer what's actually being called. aCount is required for returning arrays through XPCOM (see method signature in nsIBrowserSearchService). > @@ -2029,16 +2139,17 @@ SearchService.prototype = { > if (aVerb == SEARCH_ENGINE_LOADED) { > var engine = aEngine.QueryInterface(Ci.nsISearchEngine); > LOG("nsISearchEngine::observe: Done installation of " + engine.name > + "."); > this._addEngineToStore(engine.wrappedJSObject); > } > break; > case XPCOM_SHUTDOWN_TOPIC: > + this._saveSortedEngineList(); > > I'd rather save stuff earlier than XPCOM shutdown, can't we do this on > quit-application-requested or something a little earlier, aside from the "drop > prefs as impl detail" bit. I'm going to address that in bug 335101, which I also want to fix as soon as this lands. > + var os = Cc["@mozilla.org/observer-service;1"]. > + getService(Ci.nsIObserverService); > + os.addObserver(this, "browser-search-engine-modified", > + false); > > why the linebreak for this? Because it used to be somewhere else that required it :). fixed that. > + /** > + * Moves the selected engine either up or down in the engine list > + * @param aDir > + * -1 to move the selected engine down, +1 to move it up. > + */ > + move: function engineManager_move(aDir) { > + var selectedEngine = gEngineView.selectedEngine; > + if (!selectedEngine) > + return; > + > + if (Math.abs(aDir) != 1) > + throw new Error("Invalid direction!"); > + > + var newIndex = gEngineView.selectedIndex - aDir; > + > + if (newIndex < 0 || newIndex > gEngineView.lastIndex) > + throw new Error("Invalid aDir!"); > + > + gEngineView._engineStore.moveEngine(selectedEngine, aDir); > + > + gEngineView.invalidate(); > + gEngineView.selection.select(newIndex); > + }, > + > + onSelect: function engineManager_onSelect() { > + var noEngineSelected = (gEngineView.selectedIndex == -1); > + document.getElementById("remove").disabled = noEngineSelected; > + // document.getElementById("rename").disabled = noEngineSelected; > + document.getElementById("alias").disabled = noEngineSelected; > + > + document.getElementById("up").disabled = noEngineSelected || > + (gEngineView.selectedIndex == 0); > + > + var lastSelected = (gEngineView.selectedIndex == gEngineView.lastIndex); > + document.getElementById("dn").disabled = noEngineSelected || > + lastSelected; > + } > +}; > > I think this would be nicer if it supported move to top/move to bottom and dnd > (i.e. more than one step at a time) Filed bug 335086 for drag&drop, and created moveToIndex on the dialog. Do you think the dialog should have top/bottom buttons? > > Index: browser/components/search/content/engineManager.xul > no boilerplate! fixed > +<?xml-stylesheet href="chrome://global/skin/"?> > + > +<!DOCTYPE dialog SYSTEM "chrome://browser/locale/engineManager.dtd"> > + > +<dialog id="engineManager" > + xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" > + buttons="accept,cancel" > + onload="gEngineManagerDialog.init();" > + ondialogaccept="gEngineManagerDialog.onOK();" > + title="&engineManager.title;" > + style="min-width: 40em;" > + windowtype="Browser:SearchManager"> > > localize the minwidth! fixed. > + <label>&engineManager.intro;</label> > > <description> please, so this wraps nicely if and when the string is more > verbose fixed > + <hbox flex="1"> > + <tree id="engineList" flex="1" rows="10" hidecolumnpicker="true" > + seltype="single" onselect="gEngineManagerDialog.onSelect();"> > + <treechildren id="engineChildren" flex="1"/> > + <treecols> > + <treecol id="engineName" class="sortDirectionIndicator" flex="3" > + persist="width" label="&nameColumn.label;"/> > + <splitter class="tree-splitter"/> > + <treecol id="engineAlias" class="sortDirectionIndicator" flex="2" > + persist="width" label="&aliasColumn.label;"/> > + </treecols> > + </tree> > + <vbox> > + <spacer flex="1"/> > + <button id="up" label="&up.label;" accesskey="&up.accesskey;" > + oncommand="gEngineManagerDialog.move(1);"/> > + <button id="dn" label="&dn.label;" accesskey="&dn.accesskey;" > + oncommand="gEngineManagerDialog.move(-1);"/> > + <spacer flex="1"/> > + <button id="alias" label="&editAlias.label;" > + accesskey="&editAlias.accesskey;" > + oncommand="gEngineManagerDialog.editAlias();"/> > + <button id="remove" label="&delete.label;" > + accesskey="&delete.accesskey;" > + oncommand="gEngineManagerDialog.remove();"/> > > These should be commands instead of oncommand, so we can skip sanity checks in > the commands in favour of disabling the command when appropriate (also, > better/expected UI to do this) Not sure what you mean by "also, better/expected UI to do this". I added <command>s and fixed onSelect accordingly. > Index: browser/locales/en-US/chrome/browser/searchbar.dtd > +<!ENTITY cmd_engineManager.label "Manage search engines..."> > +<!ENTITY cmd_engineManager.accesskey "M"> > > Should be Manage Search Engines fixed also fixed: - made dialog non-resizable - made dialog position persistent - fixed focus outline of "Add engines" by adding a spacer - open the dialog on a timeout to avoid modality weirdness (dropdown opening when making changes) Let me know if you think any of the dependent bugs (bug 335086, bug 335101, bug 335102) need to be fixed before this lands and I can roll them in to this patch.

Hmm, Bugzilla's interdiff isn't all that useful. I made http://gavinsharp.com/tmp/232272-v1v2.html if that helps!

Comment on attachment 219466 [details] [diff] [review] search engine manager, v1.1 Good work, Gavin. Some comments that can be addressed in polish and shine bugs for B1: - the whole "alias" thing seems really power-userish and confuses what should be a simple UI; I'd recommend just removing it until there are some clear use cases behind it. This should also result in making the dialog narrower. - s/Add Search Engines .../Get more search engines.../; also, the label should be moved down a little, it looks too tight up against the textbox like that. - the "Get more search engines" button should close the dialog and load the new page as per the user's new window prefs (ie: either in a new tab or a new window). I've been meaning to file this against the EM as well, fwiw. :) - s/The following Search Engines are available/You have the following search engines installed: - I think you have it in another bug, but a "Restore Default Set" button is needed; just wanted to get that on record - I also notice that you're planning on adding drag and drop support; nice, nice

(In reply to comment #82) > (From update of attachment 219466 [details] [diff] [review] [edit]) > - the whole "alias" thing seems really power-userish and confuses what should > be a simple UI; I'd recommend just removing it until there are some clear use > cases behind it. This should also result in making the dialog narrower. removed the alias stuff (at least for now). > - s/Add Search Engines .../Get more search engines.../; also, the label should > be moved down a little, it looks too tight up against the textbox like that. Made the text change, but I can't really align the link with the buttons, given how the dialog binding works. I'll try and see if I can fix this some other way. > - the "Get more search engines" button should close the dialog and load the new > page as per the user's new window prefs (ie: either in a new tab or a new > window). I've been meaning to file this against the EM as well, fwiw. :) Fixed that (EM is bug 262575, we need a more generic solution to this problem). > - s/The following Search Engines are available/You have the following search > engines installed: fixed > - I think you have it in another bug, but a "Restore Default Set" button is > needed; just wanted to get that on record Yep, bug 334486. > - I also notice that you're planning on adding drag and drop support; nice, > nice (yep, bug 335086)

http://gavinsharp.com/tmp/232272-v2v3.html - interdiff between v2 and v3 (UI review comments) http://gavinsharp.com/tmp/232272-v1v3.html - interdiff between v1 and v3

Comment on attachment 219919 [details] [diff] [review] search engine manager, v1.2 >Index: browser/components/search/content/engineManager.xul >+ <script type="application/x-javascript" >+ src="chrome://browser/content/search/engineManager.js"/> >+ <stringbundle id="engineManagerBundle" >+ src="chrome://browser/locale/search.properties"/> This stringbundle isn't used, I've removed it locally.

Comment on attachment 219919 [details] [diff] [review] search engine manager, v1.2 Ok, looks good to go!

Checked in, 1.8 branch and trunk.

(In reply to comment #82) > - the "Get more search engines" button should close the dialog and load the new > page as per the user's new window prefs (ie: either in a new tab or a new > window). I've been meaning to file this against the EM as well, fwiw. :) Bug 262575 will first have to be fixed to do this for the EM since the EM is a toolkit app and the app isn't always a browser. Basically, we have widgets in toolkit that the EM uses (e.g. label class text-link) that don't know how to handle this and toolkit components in general have no way to accomplish this today generically.

Currently there is no padding between the text and the icon in the manager. Could this be fixed?

(In reply to comment #89) > Currently there is no padding between the text and the icon in the manager. > Could this be fixed? Yeah, I noticed that earlier, see comment 68. I filed bug 335820 for that, and bug 335822 for improving the UI in the "no engines installed" case.

Why not add the search engine manager to the new Tools > "Add-ons" menu?

(In reply to comment #91) > Why not add the search engine manager to the new Tools > "Add-ons" menu? > -> Bug 335781

*** Bug 318126 has been marked as a duplicate of this bug. ***