329281 - 'Add Keyword for this Search' does not work w/ Places

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect, P3)

Description

[Richard Shin]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060303 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060303 Firefox/1.6a1

When I right-click an input field and select 'Add Keyword for this Search' (eg., on search forms) nothing happens.

Reproducible: Always

Steps to Reproduce:
1. Go to a site with suitable form (eg. a search form)
2. Right click and select 'Add Keyword for this Search'
Actual Results:  
Nothing happens

Expected Results:  
A dialog should appear

Comment 1

[Dão Gottwald [:dao]]

Keyword searches doesn't work for me at all with Places.

Comment 2

[Brett Wilson]

See bug 318817 for imported keyword searches to work. This bug is only for the add search menu item.

Comment 3

[Phil Ringnalda (:philor)]

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

Comment 4

[Peter Kasting]

This annoys me, so I guess I'll take it.

From brettw via email:

"You need to call setKeywordForURI on the bookmarks service, see
http://lxr.mozilla.org/seamonkey/source/toolkit/components/places/public/nsINavBookmarksService.idl

You can get the bookmarks service by following the directions in history:
http://developer.mozilla.org/en/docs/Places:History_Service
and replacing NavHistoryService/nav-history-service with
NavBookmarksService/nav-bookmarks-service"

Comment 5

[Peter Kasting]

I'm going to dump this back off my plate for dietrich/sspitzer et. al to handle.

Comment 6

[Dietrich Ayala (:dietrich)]

Attachment: fix v1

this patch:

- gets keywords working again (were broken from removing the bookmarksService global)
- allows empty titles in the bookmarks dialog when adding
- fixes this bug
- adds keyword population to showAddBookmarkUI and the bookmark properties dialog

Comment 7

[Dietrich Ayala (:dietrich)]

adding as a blocker b/c smart keywords are broken until this patch is checked in.

Comment 8

[Asaf Romano (gone)]

Comment on attachment 259133 [details] [diff] [review]
fix v1

>Index: browser/components/places/content/bookmarkProperties.js
>===================================================================

>@@ -191,30 +192,33 @@ var BookmarkPropertiesPanel = {
>         case "bookmark":
>           this._action = ACTION_ADD;
>           this._itemType = BOOKMARK_ITEM;
>           if ("uri" in dialogInfo) {
>             NS_ASSERT(dialogInfo.uri instanceof Ci.nsIURI,
>                       "uri property should be a uri object");
>             this._bookmarkURI = dialogInfo.uri;
>           }
>-          if (!this._itemTitle) {
>+          if (this._itemTitle == null) {

I kinda prefer |if (typeof(this.itemTitle) != "string")|.

> 
>-      if (this._bookmarkKeyword)
>+      if (this._bookmarkKeyword != null)

ditto.

>Index: browser/components/places/content/utils.js
>===================================================================
>    */
>   showAddBookmarkUI: function PU_showAddBookmarkUI(aURI,
>                                                    aTitle,
>                                                    aDescription,
>                                                    aDefaultInsertionPoint,
>                                                    aShowPicker,
>-                                                   aLoadInSidebar) {
>+                                                   aLoadInSidebar,
>+                                                   aKeyword) {

keep the argument list in sync with the new showMinimal* method.

>     }
> 
>     // allow default empty title
>     if (typeof(aTitle) == "string")
>       info.title = aTitle;
> 
>     if (aDescription)
>@@ -709,16 +715,21 @@ var PlacesUtils = {
>       info.defaultInsertionPoint = aDefaultInsertionPoint;
>       if (!aShowPicker)
>         info.hiddenRows.push("folder picker");
>     }
> 
>     if (aLoadInSidebar)
>       info.loadBookmarkInSidebar = true;
> 
>+    if (aKeyword != null)
>+      info.keyword = aKeyword;
>+    else
>+      info.hiddenRows.push("keyword");
>+

So this should only be done in the new showMinimal* version of this method.

Comment 9

[Dietrich Ayala (:dietrich)]

Attachment: fix v2 - fixes mano's comments

Comment 10

[Asaf Romano (gone)]

Comment on attachment 259930 [details] [diff] [review]
fix v2 - fixes mano's comments

>Index: browser/components/places/content/utils.js
>===================================================================
>    * @return true if any transaction has been performed.
>    *
>    * Notes:
>-   *  - the location, description, keyword and "load in sidebar" fields are
>+   *  - the location, description and "load in sidebar" fields are
>    *    visible only if there is no initial URI (aURI is null).
>+   *  - The keyword field will be visible only if the aKeyword parameter
>+   *    was used.
>    *  - When aDefaultInsertionPoint is not set, the dialog defaults to the
>    *    bookmarks root folder.
>    */
>   showAddBookmarkUI: function PU_showAddBookmarkUI(aURI,

This is only correct for the showMinimal* version of this method, so move it over please.

r=mano otherwise.

Comment 11

[Dietrich Ayala (:dietrich)]

Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.775; previous revision: 1.774
done
Checking in browser/components/places/content/bookmarkProperties.js;
/cvsroot/mozilla/browser/components/places/content/bookmarkProperties.js,v  <--  bookmarkProperties.js
new revision: 1.40; previous revision: 1.39
done
Checking in browser/components/places/content/utils.js;
/cvsroot/mozilla/browser/components/places/content/utils.js,v  <--  utils.js
new revision: 1.27; previous revision: 1.26
done

Comment 12

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv