329842 - Add support for POST keywords

Status: VERIFIED FIXED

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

Description

[Brett Wilson]

See bug 318817

Comment 1

[Brett Wilson]

Bug 261124 is the new search service, which may be used to handle keyword searches in the future. If that doesn't work out, we'll do this bug instead for FF2.

Comment 2

[Ria Klaassen (not reading all bugmail)]

I suppose this is the bug for the broken post keyword search in trunk.

Comment 3

[Dietrich Ayala (:dietrich)]

Attachment: fix v1

stores anno against URI, as it's not specific to the bookmark, but to the URI. also, not unsetting on undo-create-bookmark, b/c again, it's not specific to the bookmark, and it'll get removed when the URI gets removed from moz_places.

Comment 4

[Asaf Romano (gone)]

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

I disagree; setting the annotation on the bookmarked URI is fine, but undoing a command should undo everything it did. For code sanity & readability, you may want to aggregate a "edit keyword" transaction w/ PlacesCreateItemTransaction rather than passing the keyword & post data to PlacesCreateItemTransaction/

Comment 5

[Dietrich Ayala (:dietrich)]

Attachment: fix v2 - fixes mano's comments

Comment 6

[Asaf Romano (gone)]

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

Aggregate !- childTransacrtions. As in,  s/PlacesEditBookmarkKeywordTransaction/PlacesEditURLKeywordTransaction

and make it take a url rather than a bookmark identifier.

Comment 7

[Dietrich Ayala (:dietrich)]

(In reply to comment #6)
> (From update of attachment 262313 [details] [diff] [review])
> Aggregate !- childTransacrtions. As in, 
> s/PlacesEditBookmarkKeywordTransaction/PlacesEditURLKeywordTransaction
> 
> and make it take a url rather than a bookmark identifier.
> 

mano, do you mean PlacesEditURLPOSTDataTransaction? the keyword is associated with the bookmark, the post data with the url.

Comment 8

[Asaf Romano (gone)]

Ew, why the difference? I somehow read it like you're making both uri-based (kinda makes sense).

Comment 9

[Dietrich Ayala (:dietrich)]

(In reply to comment #8)
> Ew, why the difference? I somehow read it like you're making both uri-based
> (kinda makes sense).
> 

keywords don't make sense to be bookmark-centric at all, as a keyword can only resolve to a single uri. however, solving that problem is not for this bug. having the post data be bookmark-centric doesn't really buy us anything, just complicates the keywords api (eg: having GetKeywordForURI have 2 out parameters or something). the current patch behaves like the old bookmarks, but allows us to eventually move towards uri-centric keywords if we want, w/o breaking the navbookmarksservice keywords api.

Comment 10

[Asaf Romano (gone)]

Attachment: patch

with import/export support.

Comment 11

[(not reading, please use [e-mail] instead)]

Comment on attachment 264812 [details] [diff] [review]
patch

as discussed over irc, please fix "this.annoataions" (and re-test undo/redo)

Comment 12

[Asaf Romano (gone)]

Attachment: for checkin

Comment 13

[Asaf Romano (gone)]

mozilla/browser/base/content/browser.js 1.784
mozilla/browser/components/places/content/bookmarkProperties.js 1.47
mozilla/browser/components/places/content/controller.js 1.150
mozilla/browser/components/places/content/utils.js 1.37
mozilla/browser/components/places/src/nsPlacesImportExportService.cpp 1.7

Comment 14

[Phil Ringnalda (:philor)]

Bug 387833 says "I can has testcase, plz?"

Comment 15

[Al Billings [:abillings - ex-MoCo]]

387833 is verified. Does that mean that this is verified as well?

That was verified in Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a7pre) Gecko/200707120404 Minefield/3.0a7pre

Comment 16

[Al Billings [:abillings - ex-MoCo]]

This has been verified and Tracy added a test case for this.

Comment 17

[Will Levine]

This works now, but POST keyword bookmarks still don't get imported properly from earlier Firefox versions.

Comment 18

[(not reading, please use [e-mail] instead)]

> This works now, but POST keyword bookmarks still don't get imported properly
> from earlier Firefox versions.

zoinks.  spun that off to bug #390747

Comment 19

[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