516637 - Remove download manager search feature

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

Remove the UI and related code. We will likely revisit this feature after 1.0

Comment 3

[:Gavin Sharp [email: [e-mail]]]

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

>diff --git a/chrome/content/downloads.js b/chrome/content/downloads.js

>-      // Make the item and add it to the end if it's active or matches the search
>+      // Make the item and add it to the end
>       let item = this._createItem(attrs);
>-      if (item && (isActive || this._matchesSearch(item))) {
>-        // Add item to the end
>-        this._list.appendChild(item);
>-      }
>-      else {
>-        // We didn't add an item, so bump up the number of items to process, but
>-        // not a whole number so that we eventually do pause for a chunk break
>-        aNumItems += .9;
>-      }
>+      this._list.appendChild(item);

I don't understand why removing the isActive check is OK (though this code is hard enough to follow that I'm probably missing something).

>-      this._initStatement(mode.value);

Much of _initStatement is now unused given that we only call it without aMode, so it could be simplified and/or inlined.

Looks good otherwise.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #3)
> (From update of attachment 400661 [details] [diff] [review])

> I don't understand why removing the isActive check is OK (though this code is
> hard enough to follow that I'm probably missing something).

The check was a bit weird: _createItem can't really fail and without a search, all items will match. The isActive (which was used to add items that were downloading, but not matching the current search) wasn't needed anymore.

> Much of _initStatement is now unused given that we only call it without aMode,
> so it could be simplified and/or inlined.

I can look into that.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

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

ok, r=me with _initStatement cleaned up or a followup filed.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

I cleaned up _initStatment and pushed:
https://hg.mozilla.org/mobile-browser/rev/cb8d30f915c4

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

verified on my n810 with 20090921 1.9.2 nightly build

Comment 8

[Dão Gottwald [:dao]]

bugspam