710437 - Add a Keyword for this Search does not warn on duplicates

Status: RESOLVED WORKSFORME

(Firefox :: Search, defect)

Description

[Tanvi Vyas[:tanvi]]

Here are 3 scenarios where I see a bug in the Keyword logic:


1) Using Manage Search Engines, set a keyword "g" for google search. Set a keyword for a search box on a webpage by right clicking and selecting "Add a Keyword for this Search".  Add the keyword "g".  Now go back to Manage Search Engines and you will see that your keyword for Google search is now gone.  Typing "g foo" into the location bar will not take you to google, but to the website you set the keyword.

2) Using Manage Search Engines, set a keyword "g" for google search.  Now set the same keyword for yahoo search.  There is no warning that the keyword is already in use, and the keyword is NOT set for yahoo.

3) Set a keyword for a search box on a webpage by right clicking and selecting "Add a Keyword for this Search".  Add a keyword "g".  Now go to Manage Search Engines and try to add the keyword "g" for google.  There is no warning that the keyword is already in use and the keyword is NOT set for google.

It looks like there is code for prompting an alert saying "You have chosen a keyword that is currently in use by a bookmark.  Please select another."  Either there is a bug in this code, or it is not complete yet.

Here are references to the files:
http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/engineManager.properties

http://mxr.mozilla.org/mozilla-central/source/browser/components/search/content/engineManager.js#188

I observe this behavior using both firefox 8 and the nightly build from mozilla-central.  Thanks!

Comment 1

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

(In reply to Tanvi Vyas from comment #0)
> It looks like there is code for prompting an alert saying "You have chosen a
> keyword that is currently in use by a bookmark.  Please select another." 
> Either there is a bug in this code, or it is not complete yet.

It should be complete, but there indeed could be a bug. Bug 709589's attachment 580762 [details] [diff] [review] has a suggested fix that might explain problems 2) and 3).

1) would be a different issue, though - we don't check search engine aliases when adding keyword bookmarks, and the fact that in your testing it appears to clear the alias is surprising to me.

Comment 2

[Tanvi Vyas[:tanvi]]

I applied the patch from Bug 709589's attachment 580762 [details] [diff] [review] [diff] [details] [review] and it fixes issues 1, 2, and 3 with one caveat.

When you add a keyword to a bookmark that is already in use by a Search Engine, there is no alert to the user.  The keyword continues to work for the Search Engine, and NOT the webpage you set the bookmark on.  This behavior seems right, it's just missing the alert to the user.

Comment 3

[Matthew N. [:MattN]]

(In reply to Tanvi Vyas from comment #2)
> When you add a keyword to a bookmark that is already in use by a Search
> Engine, there is no alert to the user.

Adding a warning for that case is bug 386059.

Comment 4

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

Sounds like bug 709589 fixed the problem described in this bug's summary, and the other mentioned issues are tracked in other bugs.