Closed Bug 329281 Opened 14 years ago Closed 13 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: scotshin, Assigned: dietrich)

References

()

Details

Attachments

(1 file, 1 obsolete file)

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
Keyword searches doesn't work for me at all with Places.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Target Milestone: --- → Firefox 2 beta1
See bug 318817 for imported keyword searches to work. This bug is only for the add search menu item.
OS: Windows XP → All
Hardware: PC → All
Depends on: 329842
No longer depends on: 329842
*** Bug 334068 has been marked as a duplicate of this bug. ***
Target Milestone: Firefox 2 beta1 → ---
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"
Assignee: nobody → pkasting
Blocks: 317881
I'm going to dump this back off my plate for dietrich/sspitzer et. al to handle.
Assignee: pkasting → nobody
Attached patch fix v1 (obsolete) — Splinter Review
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
Assignee: nobody → dietrich
Status: NEW → ASSIGNED
Attachment #259133 - Flags: review?(mano)
adding as a blocker b/c smart keywords are broken until this patch is checked in.
Blocks: 370099
No longer blocks: 317881
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.
Attachment #259133 - Flags: review?(mano) → review-
Attachment #259133 - Attachment is obsolete: true
Attachment #259930 - Flags: review?(mano)
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.
Attachment #259930 - Flags: review?(mano) → review+
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
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.