419403 - Pimp My Download Manager Search! (multi word, match any text)

Status: VERIFIED FIXED

(Toolkit :: Downloads API, defect)

Description

[Ed Lee :Mardak]

Attachment: screenshot of v1

Instead of only matching the search phrase against the download file name or download source/referrer, we can match against any text shown to the user including the date/time (time, yesterday, weekday, month), download status (canceled, failed, etc.), or file size.

Additionally, instead of matching just the phrase, we can do multi-word search that matches against the site and name at the same time. E.g., "mozilla.org filename".

Attached is a screenshot of the multi-word search matching the name, completed file size, completed time, and referrer eTLD+1.

Comment 1

[Ed Lee :Mardak]

Attachment: v1

This also fixes bug 392536 instead of always removing a just-finished download if we're searching.

sdwilsh: :( No more createFunction.

Comment 2

[Ed Lee :Mardak]

Attachment: v1.1

Whoops.. would help to save the file before qrefreshing ;)

I suppose shawn will want a testcase......

Comment 3

[Ed Lee :Mardak]

Attachment: v1.2

Zzz.. it always takes so much longer writing the test than the code to implement the feature. :p

Comment 4

[Ed Lee :Mardak]

Attachment: v1.3

Get rid of the flicker when typing a space at the end (or a space anywhere that already has a space).

Here's a build that doesn't has v1.2 (so it'll flicker when typing a space for a second word). Also, it seems like building the list when searching could go faster, so changing +.75 to +.9. The bottleneck here is adding items to the UI - not querying against the DB.

https://build.mozilla.org/tryserver-builds/2008-02-25_01:[e-mail]/

Bug 416446 - Make PluralForm more useful for extensions and delay importing strings for perf
Bug 415460 - searching in places queries does not decode urls
Bug 418961 - "Save Page As" "Text Files" saves file but Downloads window doesn't show completion
Bug 401660 - when showing autocomplete result, show tags that partially match
Bug 416211 - Tagged results don't show bookmark title when matching the tag
Bug 405320 - support "out of order" multiple word tag search in the url bar and in the places organizer
Bug 418920 - Allow multiple word search with tags in addition to title and url
Bug 419068 - Allow tagged pages to mingle with non-tagged autocomplete results
Bug 407946 - emphasize all matching text in title and url, not just the first match in title and url
Bug 415403 - Show matches for all search words for location bar autocomplete
Bug 395735 - Instrument the location bar auto-complete to report accuracy
Bug 395739 - adaptive learning (match entered text to selected item) in url bar autocomplete
Bug 392143 - show keywords as url bar autocomplete choices
Bug 407204 - adjust the title and url text sizes
Bug 406257 - reduce the number of rows in url bar autocomplete from 10 to 6
Bug 419324 - Edit bookmarks dialog doesn't unescape URLs
Bug 419403 - Pimp My Download Manager Search! (multi word, match any text)

Comment 5

[Shawn Wilsher :sdwilsh]

Comment on attachment 305552 [details] [diff] [review]
v1.3

>+  // Don't rebuild the download list if the search didn't change unless forced
>+  if (gSearchTerms.join(" ") == prevSearch && !aForceBuild)
check !aForceBuild first - it's less expensive

>+    let isActive = gStmt.getInt32(10);
ugh - we so need to start using named parameters soon.

>+ * @param aItem
>+ *        Download richlist item to check if it matches the current search
nit: richlistitem

>+  // Search through these download attributes that are shown to the user and
s/these/the/

>+  // make it into one big string for easy combined searching
>+  let combinedSearch = "";
>+  for each (let attr in ["target", "status", "dateTime"])
let's make this array a const (perhaps up top?) so it's easy to modify this behavior down the line without ruining much blame (up top makes it easier to modify too).

>+  browser_pimped_search.js \
let's call it something a bit more accurate - "browser_multiword_search.js"

r=sdwilsh

Comment 6

[Ed Lee :Mardak]

Attachment: v1.4

(In reply to comment #5)
> (From update of attachment 305552 [details] [diff] [review])
> r=sdwilsh
> >+  // Don't rebuild the download list if the search didn't change unless forced
> >+  if (gSearchTerms.join(" ") == prevSearch && !aForceBuild)
> check !aForceBuild first - it's less expensive
I had that before, but I was too lazy to change the comment ;) Fixed.

> >+ * @param aItem
> >+ *        Download richlist item to check if it matches the current search
> nit: richlistitem
De-spaced.

> >+  // Search through these download attributes that are shown to the user and
> s/these/the/
Articleized.

> >+  // make it into one big string for easy combined searching
> >+  let combinedSearch = "";
> >+  for each (let attr in ["target", "status", "dateTime"])
> let's make this array a const (perhaps up top?) so it's easy to modify this
> behavior down the line without ruining much blame (up top makes it easier to
> modify too).
Sure. Put up top with one item per line.

> >+  browser_pimped_search.js \
> let's call it something a bit more accurate - "browser_multiword_search.js"
Fine fine. ;)

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 305644 [details] [diff] [review]
v1.4

a=beltzner for 1.9

Comment 8

[Ed Lee :Mardak]

Checking in toolkit/mozapps/downloads/content/downloads.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.js,v  <--  downloads.js
new revision: 1.139; previous revision: 1.138
done
Checking in toolkit/mozapps/downloads/tests/browser/Makefile.in;
/cvsroot/mozilla/toolkit/mozapps/downloads/tests/browser/Makefile.in,v  <--  Makefile.in
new revision: 1.16; previous revision: 1.15
done
RCS file: /cvsroot/mozilla/toolkit/mozapps/downloads/tests/browser/browser_multiword_search.js,v
done
Checking in toolkit/mozapps/downloads/tests/browser/browser_multiword_search.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/tests/browser/browser_multiword_search.js,v  <--  browser_multiword_search.js
initial revision: 1.1
done

Comment 9

[Ed Lee :Mardak]

Shawn: Is it okay to disable the test on windows for now? I'll file a bug to fix the test for windows.

ifneq ($(OS_ARCH), WINNT)
_BROWSER_FILES += ...

Comment 10

[Shawn Wilsher :sdwilsh]

weird that it's only windows.  sure, r=me on that

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

https://litmus.mozilla.org/show_test.cgi?id=5197

in-litmus+

Comment 12

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008022804 Minefield/3.0b4pre

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008022806 Minefield/3.0b4pre

-and-

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b4pre) Gecko/2008022704 Minefield/3.0b4pre