browser.download.manager.addToRecentDocs isn't working; downloaded documents are not added to "My Recent Documents"

VERIFIED FIXED in mozilla1.9beta1

Status

()

P1
normal
VERIFIED FIXED
11 years ago
10 years ago

People

(Reporter: stephend, Assigned: Gavin)

Tracking

({regression, relnote})

Trunk
mozilla1.9beta1
x86
Windows XP
regression, relnote
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

11 years ago
Gavin asked that I file a new bug, instead of merely reopening bug 398174, so here it is :-)

Build ID: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007102705 Minefield/3.0a9pre

Summary: browser.download.manager.addToRecentDocs isn't working; downloaded documents are not added to "My Recent Documents"

Steps to Reproduce:

1. Using a nightly build, download some .rtf, .doc, .pdf, .txt, etc. files
2. In Windows XP, click on start -> "My Recent Documents"

Expected Results:

Files downloaded in step 1 should be populated in this flyout list

Actual Results:

(Empty)
Created attachment 286461 [details] [diff] [review]
patch

Trying to figure out how to write a test for this.
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Created attachment 286521 [details] [diff] [review]
patch
Attachment #286461 - Attachment is obsolete: true
Attachment #286521 - Flags: review?
Attachment #286521 - Flags: review? → review?(comrade693+bmo)
Flags: blocking-firefox3?
Priority: -- → P1
Target Milestone: --- → Firefox 3 M9
Comment on attachment 286521 [details] [diff] [review]
patch

>-          if (fileURL && fileURL->GetFile(getter_AddRefs(file)) && file && 
>+          if (fileURL &&
>+              NS_SUCCEEDED(fileURL->GetFile(getter_AddRefs(file))) &&
>+              file &&
out of curiosity, how exactly was this failing before?

> var gDownloadCount = 0;
>-function addDownload()
>+function addDownload(aResultFileName)
could you please add a javadoc style comment above this explaining what the function does and the argument now please?  I probably should have had that when I first landed these tests...

>+  // delete the saved file
>+  try {
>+    var resultFile = dirSvc.get("ProfD", Ci.nsIFile);
>+    resultFile.append(resultFileName);
>+    resultFile.remove(false);
>+  } catch (ex) {}
why is this in a try catch?

>+  var thread = Cc["@mozilla.org/thread-manager;1"]
>+               .getService().currentThread;
>+
>+  while (!httpserv.isStopped())
>+    thread.processNextEvent(true);
>+
>+  // get rid of any pending requests
>+  while (thread.hasPendingEvents())
>+    thread.processNextEvent(true);
In theory, we shouldn't need to do this when we use do_test_pending.  There's a bug on file to cleanup the rest of the download manager tests as well.

r=sdwilsh
Attachment #286521 - Flags: review?(comrade693+bmo) → review+
(In reply to comment #3)
> (From update of attachment 286521 [details] [diff] [review])
> >-          if (fileURL && fileURL->GetFile(getter_AddRefs(file)) && file && 
> >+          if (fileURL &&
> >+              NS_SUCCEEDED(fileURL->GetFile(getter_AddRefs(file))) &&
> >+              file &&
> out of curiosity, how exactly was this failing before?

GetFile returns NS_OK, which is 0, which mean the code was never reached.

> >+  // delete the saved file
> >+  try {
> >+    var resultFile = dirSvc.get("ProfD", Ci.nsIFile);
> >+    resultFile.append(resultFileName);
> >+    resultFile.remove(false);
> >+  } catch (ex) {}
> why is this in a try catch?

No good reason, I guess.
Attachment #286521 - Flags: approvalM9?
Created attachment 286582 [details] [diff] [review]
comments addressed
Attachment #286521 - Attachment is obsolete: true
Attachment #286582 - Flags: approvalM9?
Attachment #286521 - Flags: approvalM9?
Comment on attachment 286582 [details] [diff] [review]
comments addressed

a=drivers for after M9 freeze
Attachment #286582 - Flags: approvalM9?
Attachment #286582 - Flags: approvalM9-
Attachment #286582 - Flags: approval1.9+
Flags: blocking-firefox3? → blocking-firefox3+
Keywords: relnote
Target Milestone: Firefox 3 M9 → Firefox 3 M10
Comment on attachment 286582 [details] [diff] [review]
comments addressed

a=endgame drivers for M9 after reconsideration
Attachment #286582 - Flags: approvalM9- → approvalM9+
toolkit/components/downloads/test/unit/test_bug_401430.js 	1.1
toolkit/components/downloads/src/nsDownloadManager.cpp 	1.145
toolkit/components/downloads/test/unit/head_download_manager.js 	1.6
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 3 M10 → Firefox 3 M9
(Reporter)

Comment 9

11 years ago
Tested with tinderbox builds from ftp://ftp.mozilla.org/pub/firefox/tinderbox-builds/FX-WIN32-TBOX-trunk of:

Windows XP SP 2 - Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007103017 Minefield/3.0a9pre

and Vista - Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a9pre) Gecko/2007103017 Minefield/3.0a9pre

This works like a charm, now; in Vista, just setting the Start Menu style to "classic" makes the fix apparent.

Verified FIXED; I'll take care of bug 398174, too, since these verifications were done using VMWare Fusion, which means new profiles.
Status: RESOLVED → VERIFIED
Blocks: 381517
Keywords: regression
We're seeing a make check failure in this test on the new PGO unittest machine. The machine is a win2k3 VM building PGO and running unittests.

../../../../_tests/xpcshell-simple/test_dm/unit/test_bug_401430.js: FAIL

http://qm-unittest02.mozilla.org:2005/WINNT%205.2%20qm-win2k3-pgo01%20dep%20unit%20test/builds/52/step-check/0

Any help would be much appreciated.
That url isn't working - attach the log?
(In reply to comment #12)
> http://tinderbox.mozilla.org/showlog.cgi?log=UnitTest/1207920809.1207931950.21080.gz#err0

I added a patch in bug 420073 that should help provide more information as to why it's failing - see bug 420073 comment 31.
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.