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

RESOLVED FIXED

Status

()

P3
normal
RESOLVED FIXED
13 years ago
9 years ago

People

(Reporter: scotshin, Assigned: dietrich)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

13 years ago
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

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

Updated

13 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Target Milestone: --- → Firefox 2 beta1

Comment 2

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

Updated

13 years ago
OS: Windows XP → All
Hardware: PC → All

Updated

13 years ago
Depends on: 329842

Updated

13 years ago
No longer depends on: 329842
*** Bug 334068 has been marked as a duplicate of this bug. ***
Target Milestone: Firefox 2 beta1 → ---

Comment 4

12 years ago
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

Updated

12 years ago
Blocks: 317881

Comment 5

12 years ago
I'm going to dump this back off my plate for dietrich/sspitzer et. al to handle.
Assignee: pkasting → nobody
(Assignee)

Comment 6

12 years ago
Created attachment 259133 [details] [diff] [review]
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
Assignee: nobody → dietrich
Status: NEW → ASSIGNED
Attachment #259133 - Flags: review?(mano)
(Assignee)

Comment 7

12 years ago
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-
(Assignee)

Comment 9

12 years ago
Created attachment 259930 [details] [diff] [review]
fix v2 - fixes mano's comments
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+
(Assignee)

Comment 11

12 years ago
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
Last Resolved: 12 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.