163884 - downloadmanager.js: minValue not defined - and other errors

Status: VERIFIED FIXED

(SeaMonkey :: Download & File Handling, defect)

Description

[Wesha]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1b) Gecko/20020821
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1b) Gecko/20020821

JavaScript Console Says:

Error: minValue is not defined
Source File: chrome://communicator/content/downloadmanager/downloadmanager.js
Line: 255

Reproducible: Always

Steps to Reproduce:
1. Open JavaScript Console
2. Open Download Manager
3. Click "Remove from list"


Actual Results:  
JS error message on Java Console

Expected Results:  
No error

Comment 1

[Wesha]

Taking

Comment 2

[Wesha]

Regression from fix for bug 132013

Comment 3

[Wesha]

Attachment: proposed patch

1) fixed "minValue not defined" bug

and also a number of tiny bugs encountered while testing:

2) fixed "gDownloadView.view has no properties"
3) fixed "gDownloadView.treeBoxObject.selection has no properties"
4) now clearing status bar after deleting all downloads
5) now propely placing selection after deletion
6) now ensuring that this new selection is visible

Comment 4

[Markus Gerstel]

Adding BlakeR as default owner of the dlmanager.
Blake: Could you review the patch in this bug ?

Wesha: You probably should add the default owner to CC if you take a bug yourself.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 96234 [details] [diff] [review]
proposed patch

sr=bzbarsky if you fix the funky indentation of your closing braces.

Have you actually mailed someone asking for review?

Comment 6

[Wesha]

Attachment: Modified patch as per comment 5

Indentation of my closing braces is no longer funky. =^.^=

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 103660 [details] [diff] [review]
Modified patch as per comment 5

sr=bzbarsky

Comment 8

[jag (Peter Annema)]

Comment on attachment 103660 [details] [diff] [review]
Modified patch as per comment 5

>Index: xpfe/components/download-manager/resources/downloadmanager.js

>@@ -252,9 +256,18 @@
>       var remote = window.arguments[0].QueryInterface(Components.interfaces.nsIRDFRemoteDataSource);
>       remote.Flush();
>       gDownloadView.builder.rebuild();
>-      if (minValue >= gDownloadView.treeBoxObject.view.rowCount)
>-        --minValue;
>-      gDownloadView.treeBoxObject.selection.select(minValue);
>+      // If there's nothing on the panel now, skip setting the selection
>+      if(gDownloadView.treeBoxObject.view && gDownloadView.treeBoxObject.view.rowCount > 0) {

Space between if and (

r=jag with that nit addressed.

Comment 9

[Wesha]

Attachment: Modified patch

Nit addressed as per comment 8.

Comment 10

[Boris Zbarsky [:bzbarsky]]

There go the closing braces again... ;)

Comment 11

[Wesha]

Attachment: Modified patch as per comments 5 AND 8

Merp, I need to sit down and figure out what the hell is going on with my
PatchMaker sometime. :/ 

Fixed braces, too.

(Maybe it would make sense to add some macro expansion stuff to do such a
things automatically when ULing a patch? Like "if(" -> "if (", etc.?)

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 103732 [details] [diff] [review]
Modified patch as per comments 5 AND 8

marking the r=jag, sr=bzbarsky

Comment 13

[Asa Dotzler [:asa]]

Comment on attachment 103732 [details] [diff] [review]
Modified patch as per comments 5 AND 8

a=asa for checkin to 1.2 (on behalf of drivers)

Comment 14

[Boris Zbarsky [:bzbarsky]]

fixed.

Comment 15

[Wesha]

verified 2002111108/NT4, 2002111108/win98