200136 - smartfind bookmarks broken since landing of bug 196756 (find is not a registered protocol)

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Travis Crump]

smartfind bookmarks are bookmarks which have a url of the form
"find:datasource=rdf:bookmarks&match=http://home.netscape.com/NC-rdf#Name&method=contains&text=Mozilla".
 Examples can be found in the default bookmarks under Mozilla Project ->
Technology Demonstration -> SmartFind queries.  They are supposed to behave like
folders containing all the bookmarks matching the search parameters encoded in
the url[ie the above url encodes 'bookmarks which contain "Mozilla" in the
Name'].  They now do absolutely nothing.  They now appear to be normal
bookmarks, but when you click on them you get an error "find is not a registered
protocol".  The old behavior/functionality should be restored.  They have been
my primary method for organizing my bookmarks for the last year and a half.  I
am observing the broken functionality on a gtk[1.2]/gcc-3.2 linux cvs build from
this morning.  I first noticed it yesterday, but I can only assume that the
bookmark rewrite landing a week ago is not a coincidence.

Comment 1

[Jan Varga [:janv]]

nominating

Comment 2

[Paul Wyskoczka]

Nav triage team: nsbeta1-

Comment 3

[mozbugs]

bug 200483 and bug 205539 are unconfirmed duplicates of this bug

Comment 4

[Matthias Versen [:Matti]]

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

Comment 5

[Matthias Versen [:Matti]]

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

Comment 6

[René Rhéaume]

Why a regression bug is targeted for Mozilla 1.5 alpha? As Mozilla 1.4 will
replace 1.0.x series as the stable codebase, this bug should be corrected for
1.41 timeframe. We have at least an hint about the origin of the bug : bug
196756 fix landing.

Comment 7

[Matthias Versen [:Matti]]

Attach a patch and get reviews before they ship 1.4
nsbeta- means that the NSCP developer can't work on this for 1.4

Comment 8

[mozbugs]

If the underlying bug doesn't get fixed, one should perhaps remove the "save
query in bookmarks" feature from bookmarks search to prevent people from
reporting "my saved query does not work". At least for a longstanding release
like 1.4, this should be considered an option.

Comment 9

[Jan Varga [:janv]]

yes, the "save query in bookmarks" should be temporary removed

Comment 10

[Jan Varga [:janv]]

Attachment: patch

Comment 11

[Pierre Chanial]

are you sure the dialog box will close?
|document.getElementById("saveQuery").checked|
in findBookmarks.js will trigger an error and |find()| will not return true
In either cases, I think you should comment the related part in findBookmarks.js
and also the tech demo that uses find: in the default bookmarks.html.

Comment 12

[Jan Varga [:janv]]

Good catch, sorry for being rash.
Do we really want to remove/comment out it in the default bookmarks file also?
I know that it looks silly when they don't expand as they should, but if we
remove them in the default profile, they won't be available anymore with a
profile based on the default profile.

Comment 13

[Pierre Chanial]

I have no strong opinion on that, both have pros and cons.
over to Jag, the module owner to decide.

Comment 14

[Jan Varga [:janv]]

Jag, do you think that the smartfing queries should be removed temporary from
the default bookmarks file?

Comment 15

[mozbugs]

Will the workaround of hiding the save-as-query feature be achieved for 1.4? It
seems that there is still no decision on this or rather the removal from default
bookmarks from the module owner.

Concerning the proposed patch: how about setting the hidden-attribute on the
checkbox instead of removing it? That way one does not need to worry about the
further flow of control as long as the hidden checkbox value is "false".

Comment 16

[Eric James Soltys]

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

Comment 17

[mozbugs]

Attachment: hide the checkbox creating smartfind queries

Probably too late for 1.5, but this is a patch hiding the checkbox in the
search bookmarks dialog. It will prevent users from creating broken smartfind
queries.

Comment 18

[mozbugs]

Comment on attachment 131452 [details] [diff] [review]
hide the checkbox creating smartfind queries

Requesting r= from Jan Varga.

Who should I ask for sr=? (This is my first patch...)

Comment 19

[Andy Edmonds]

Whiteboard: haspatch.  Noting blockage of publicize.

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 131452 [details] [diff] [review]
hide the checkbox creating smartfind queries

I'm not a big fan of this approach -- if a feature doesn't work anymore, it
should either be removed or fixed -- we shouldn't ship extra code that doesn't
do anything.

How hard is it to fix the real problem?  Do people use the feature?

Comment 21

[Daniel Wang]

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

Comment 22

[Hermann Schwab]

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

Comment 23

[Rupesh]

Attachment: Proposed Patch

Can somebody try this patch?

Comment 24

[Giacomo Magnini]

(In reply to comment #23)
> Can somebody try this patch?

I've tried it with the supplied string in comment 0 with no luck, still not a
registered protocol...

Comment 25

[Rupesh]

Attachment: Updated Patch

I created the patch from Mozilla 1.7.7 on OS/2 source and it works for me from
Bookmarks Manager window. To make it work from the Navigator window also, we
need to add the below line to navigator.xul
---------
  <script type="application/x-javascript"
src="chrome://communicator/content/bookmarks/findBookmark.js"/>  
---------
I am giving the updated patch here.

Comment 26

[Muni Sekhar Reddy]

I tried patch given in Comment # 25. It is working for me.  This updated patch
may be tried now.

Comment 27

[Giacomo Magnini]

Works like a charm in Bookmarks Manager, but it doesn't in Navigator window for
me: maybe I did something wrong? (trunk build, linux)

Comment 28

[Rupesh]

(In reply to comment #27)
> Works like a charm in Bookmarks Manager, but it doesn't in Navigator window for
> me: maybe I did something wrong? (trunk build, linux)

   Do you see any javascript error? FYI, I verified it on Windows Nightly build
"Gecko/20050518" and found no issues.

Comment 29

[Giacomo Magnini]

My fault. It works also in Navigator. Please request r/sr=? and get this in! :)

Comment 30

[Rupesh]

Hello Jan,
         Could you review the patch provided for this problem?

Comment 31

[Giacomo Magnini]

(In reply to comment #30)
> Hello Jan,
>          Could you review the patch provided for this problem?

Please use r/sr ? flag in the Edit Attachment page to ask for r/sr: many peers
don't even read bugzilla mails, and this will allow for better visibility...
Ask neil.parkwaycc for sr.

Comment 32

[Rupesh]

Attachment: Working Patch

Comment 33

[[e-mail]]

As I recall in the dim and distant past before the bookmarks service was
rewritten, these queries actually worked like dynamic bookmark folders.
Porting patch(es) for bug 235665 should hopefully restore this properly.

Comment 34

[Jan Varga [:janv]]

(In reply to comment #33)
> As I recall in the dim and distant past before the bookmarks service was
> rewritten, these queries actually worked like dynamic bookmark folders.
> Porting patch(es) for bug 235665 should hopefully restore this properly.

Exactly, it's already fixed in Firefox.
Rupesh, do you valunteer to port the patch
https://bugzilla.mozilla.org/attachment.cgi?id=156399 to seamonkey?

Comment 35

[Jens Hatlak (:InvisibleSmiley)]

KaiRo, Neil: Isn't this whole bug (and more specifically the approach from comments 32-34, including the sr request) obsolete now we're moving to Places-based bookmarks? I'd guess that RDF query URLs won't work there anyway and this bug is pretty specific to that.

If someone would want to have "query bookmarks" with Places, that should IMHO go to its own set of bugs (adding support to the Toolkit back-end plus FF/SM front-ends).

Comment 36

[Robert Kaiser]

The bug itself is valid, the patch probably not any more. Saved bookmarks queries will work with the places implementation, so I'm marking this depending on that other one.

Comment 37

[Robert Kaiser]

I'm marking this fixed by bug 498596 as it makes saved searches work (even if in the new places way) in current nightlies, SeaMonkey 2.1 Alpha 3 and later.