162542 - Quick Search bar for Bookmarks Manager

Status: VERIFIED FIXED

(SeaMonkey :: Bookmarks & History, enhancement)

Description

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

RFE:  quick search of bookmarks

one of the reasons this is cool is I'd no longer have to organize my bookmarks.

I could add at will, and use QS to find stuff.

like addressbook and the addressbook sidebar panel, both bookmarks manager 
dialog and the bookmarks sidebar panel would have QS.

Comment 1

[Daniel Wang]

since this RFE doesn't specify how this feature can be added, I'll
just mark it as dupe of bug 137794 (Add Search Bookmarks, Search
History to browser search menu)

reporter, for now you can search bookmarks by clicking "search" button
in the bookmarks sidebar channel. Or you can do so via custom bookmarks
(see the "Technology Demonstration" bookmark folder. If you can't see
it, create a new user profile and you will see it.)

*** This bug has been marked as a duplicate of 137794 ***

Comment 2

[Tuukka Tolvanen (sp3000)]

I don't agree with that resolution; this is a nice clean specific rfe for
quicksearch to match mailnews & addressbook (quite specifically same as bug
137794 comment 7) whereas bug 137794 is something about (menu) access points and
invocation specifics for current search functionality... or something...?

Comment 3

[Ben Bucksch (:BenB)]

I agree, no dup.

But I don't see a need for quicksearch in the bookmarks manager. the poinf ot QS
in Mailnews is that it is not in another window (like BM), but right in the app
window (like Navigator). I hope you don't want to add bookmark QS to the main
win of navigator.

Also, full Mailnews search is *way* more complex. it'd be
quite easy to add a way to search across fields in the bookmarks using the
normal bookmarks search. In that case, i'd have to be easily accessible, though,
which is (dependant on) bug 137794.

Comment 4

[Tuukka Tolvanen (sp3000)]

BenB> I hope you don't want to add bookmark QS to the main win of navigator.

See comment 0: "bookmarks manager dialog and the bookmarks sidebar panel would
have QS."

Let me just say that this here idea rocks - currently searching bookmarks for a
title string and loading a page from the results requires
  - opening 3 (!) windows (sidebar counts as a window here if going that route)
  - doing 5 actions (menu use, and search string typing count as 1)

With this rfe implemented, this would be reduced to
  - 1 window
  - 3 actions

Also, perhaps more importantly, narrowing / otherwise editing a search (as if
you ever knew what the hell you are looking for exactly beforehand):
Current:
  - 1 window
  - 3 actions

Qs:
  - 0 windows
  - 1 action

:)

Comment 5

[Bruno Abdonzilla]

the opera browser has qs of bookmarks and this feature surelly rocks. it could
be implemented as a simple search field in the bookmark side tab that filters
the bookmarks shown below it as you type your pattern.
as an opera migrant, i had found the search feature in mozilla's bookmark quite
'semi-useless'

Comment 6

[Daniel Wang]

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

Comment 7

[Jan Varga [:janv]]

taking

Comment 8

[Jan Varga [:janv]]

Attachment: patch

patch ported from Phoenix + sidebar integration

Comment 9

[Jan Varga [:janv]]

Attachment: bookmarks manager

Comment 10

[Jan Varga [:janv]]

Attachment: bookmarks sidebar

Comment 11

[Jan Varga [:janv]]

Attachment: additional patch

additional patch to correctly collapse scrollbar when there is no match

Comment 12

[Bruno Abdonzilla]

good job!

...but it would be even better if it could search bookmarks itens by url,
description and/or keyword, instead of just by name. 

in the ``find bookmarks` window, one choses between name - location - desc -
keyword to search within. i dont believe such a combo in the sidebar would be a
great idea. a simpler solution would be just to _always_ search by name desc and
keyword, which is how opera works (i guess).

Comment 13

[Pierre Chanial]

I added the currentIndex=-1 line to prevent a hang (maybe your last patch fixes
it, I haven't checked).
It's a work around and it would be nice if you could bring the real fix.

Comment 14

[Jan Varga [:janv]]

Attachment: new patch

Removed |selectedIndex = -1| + statusbar fix

Comment 15

[Jan Varga [:janv]]

bruno, yeah it would be even better, but search component only supports
searching by one property.

pch, I removed that line and I don't see any hang, even w/o content/ fix

Comment 16

[Bruno Abdonzilla]

> ...but search component only supports searching by one property

what a pitty.but still, what would be the best way out? (a) open a rfe for the
owner of the seach component, (b) make the qs do many searches and merge the
results or (c) forget about it? I'm more or less inclined towards (a), since (b)
would be huge ugly workaround.

Comment 17

[Jan Varga [:janv]]

a) sounds good

Comment 18

[Bruno Abdonzilla]

i had taken a look and it seems that bug #72799 ask exactly for multi-field
search in bookmarks. now the bad news: the last entry in the bug dates back to
oct 2001 and it says something like 'not any time soon'. maybe a whole year isnt
exactly 'soon' so we could press the guy ('ben', i guess) to have a look at it
again...

Comment 19

[Peter Trudelle]

nsbeta1+

Comment 20

[Jan Varga [:janv]]

Jag, does it look good to you ?
I mean a new textbox binding.

Comment 21

[jag (Peter Annema)]

Sure, it's a clean way to add this functionality. Looks good, sr=jag.

Comment 22

[Jan Varga [:janv]]

pch: yeah, "additional patch" fixes the problem with selection too

Comment 23

[Jan Varga [:janv]]

Attachment: fixing nits proposed by timeless

Comment 24

[Jan Varga [:janv]]

Comment on attachment 103864 [details] [diff] [review]
fixing nits proposed by timeless

r=timeless, sr=jag

Comment 25

[Jan Varga [:janv]]

checked in

Comment 26

[[e-mail]]

Comment on attachment 103864 [details] [diff] [review]
fixing nits proposed by timeless

>Index: xpfe/global/resources/content/bindings/textbox.xml
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/global/resources/content/bindings/textbox.xml,v
>retrieving revision 1.16
>diff -u -r1.16 textbox.xml
>--- xpfe/global/resources/content/bindings/textbox.xml	16 Oct 2002 22:22:07 -0000	1.16
>+++ xpfe/global/resources/content/bindings/textbox.xml	23 Oct 2002 16:22:18 -0000
>@@ -100,6 +100,42 @@
>     </handlers>    
>   </binding>
> 
>+  <binding id="timed-textbox" extends="chrome://global/content/bindings/textbox.xml#textbox">
>+    <content>
>+      <xul:hbox class="textbox-input-box" flex="1">
>+        <html:input class="textbox-input" flex="1" anonid="input"
>+                    xbl:inherits="onfocus,onblur,value,type,maxlength,disabled,size,readonly,tabindex"/>
>+      </xul:hbox>
>+    </content>
Was it necessary to duplicate the content, instead of inheriting it?

>+    <implementation>
>+      <field name="_timer">null</field>
>+      <property name="callback"
>+                onset="this.setAttribute('callback', val); return val;"
>+                onget="return this.getAttribute('callback');"/>
Shouldn't this use a command event, rather than eval(!) of inline script?

>+      <property name="timeout"
>+                onset="this.setAttribute('timeout', val); return val;"
>+                onget="var t = parseInt(this.getAttribute('timeout')); return t ? t : 0;"/>
(parseInt(...) || 0; also works)

>+    </implementation>    
>+    <handlers>
>+      <handler event="input">
>+        <![CDATA[
>+          if (this._timer)
>+            clearTimeout(this._timer);
>+          this._timer = setTimeout(this.callback, this.timeout);
>+        ]]>
>+      </handler>
>+      <handler event="keypress">
>+        <![CDATA[
>+          if (event.keyCode == 13) {
>+            if (this._timer)
>+              clearTimeout(this._timer);
>+            eval(this.callback);
Becomes this.doCommand();

>+          }
>+        ]]>
>+      </handler>
>+    </handlers>
>+  </binding>
>+
>   <binding id="textarea" extends="chrome://global/content/bindings/textbox.xml#textbox">
>     <content>
>       <xul:hbox class="textbox-input-box" flex="1">

Comment 27

[Jan Varga [:janv]]

good catch Neil, command event is better than eval

Comment 28

[jag (Peter Annema)]

I thought the content was overridden to get rid of some other anonymous content
that wasn't needed.

Comment 29

[Jan Varga [:janv]]

Attachment: additional patch

this additional patch should make it better
Neil, Jag, could you take a look ?

Comment 30

[Jan Varga [:janv]]

Comment on attachment 105356 [details] [diff] [review]
additional patch

clear button doesn't work with this patch and I'll try to reuse timed textbox
in address book too

Comment 31

[Jan Varga [:janv]]

I filed bug 179050 to fix issues pointed by Neil.

Comment 32

[[e-mail]]

Comment on attachment 105356 [details] [diff] [review]
additional patch

>+          if ("onFocus" in this)
>+            this.onFocus();
What's this for?
>+          function timeoutHandler(subject) {
>+            subject.doCommand();
I don't think mTimer gets cleared otfen enough.
>       <handler event="keypress">
keycode="VK_RETURN"?

I've attached my independent patch in your new bug.

Comment 33

[Jan Varga [:janv]]

> What's this for?
take a look at mailnews implementation of quick search

Comment 34

[[e-mail]]

Bah, has anyone noticed that the F access key conflicts with the File menu?

Comment 35

[Jan Varga [:janv]]

Attachment: fix for accesskey

Comment 36

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 107425 [details] [diff] [review]
fix for accesskey

I really wish that we would use "Search" or something similar and use an "S"
accesskey.  "n" just doesn't do justice for Find, IMO....  But my opinion
doesn't count :-)

Comment 37

[Jan Varga [:janv]]

Attachment: better patch

caillon, your opinion does count

Comment 38

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 107427 [details] [diff] [review]
better patch

Groovy.

Comment 39

[sairuh (rarely reading bugmail)]

bookmarks quicksearch works fine in both bm mgr and sidebar (only with Name, as
noted earlier). vrfy'ing with 2003.02.19 comm trunk builds.

Comment 40

[[e-mail]]

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