Closed Bug 391870 Opened 13 years ago Closed 13 years ago

Have the download manager store the referring uri when appropriate (and use it in the front end)

Categories

(Toolkit :: Downloads API, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9alpha8

People

(Reporter: stevee, Assigned: sdwilsh)

References

Details

Attachments

(3 files, 7 obsolete files)

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a8pre) Gecko/2007081115 Minefield/3.0a8pre ID:2007081115

1. Complete a download
2. Click on the Information button. You get a date and two buttons.
   The second button opens the dir the file downloaded to.
   The first button doesn't appear to do anything (it should open the file?)

So what should the first button do?
Attached image what should it do?
Nothing right now - it will open the page where you got the download from.  I can't recall if I filed a follow-up bug for that yet.
Assignee: nobody → sdwilsh
Target Milestone: --- → Firefox 3 M8
Status: NEW → ASSIGNED
OS: Windows 2000 → All
Hardware: PC → All
Summary: Clicking the first option in the DM Information pane does nothing. → Have the download manager store the referring uri when appropriate (and use it in the front end)
Attached patch v1.0 moving nsIDownload (obsolete) — Splinter Review
Moves nsIDownload as per biesi's request.
Attachment #276540 - Flags: superreview?(cbiesinger)
Attachment #276540 - Flags: review?(cbiesinger)
Attachment #276540 - Flags: superreview?(cbiesinger)
Attachment #276540 - Flags: superreview+
Attachment #276540 - Flags: review?(cbiesinger)
Attachment #276540 - Flags: review+
Attachment #276540 - Attachment is obsolete: true
Attachment #276554 - Flags: superreview?(cbiesinger)
Attachment #276554 - Flags: review?(cbiesinger)
Depends on: 392106
Attachment #276554 - Flags: superreview?(cbiesinger)
Attachment #276554 - Flags: superreview+
Attachment #276554 - Flags: review?(cbiesinger)
Attachment #276554 - Flags: review+
Attached patch v1.0 everything (obsolete) — Splinter Review
r+sr=biesi for the bits not in toolkit.

Uploaded to buildbot try patch and I'll post a link to the builds with it shortly.
Attachment #276554 - Attachment is obsolete: true
Attachment #276592 - Flags: review?(mano)
OK, so v1.0 doesn't build on windows.  I'm trying a small fix now with the try patch server.

Mac build:
https://build.mozilla.org/tryserver-builds/25-swilsher@mozilla.com-firefox-try-mac.dmg
Linux build:
https://build.mozilla.org/tryserver-builds/25-swilsher@mozilla.com-firefox-try-linux.tar.bz2
you might want to try getting the docshell referrer property from the channel and using the nsIHttpChannel only if that failed
http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#8340

(you still need the httpchannel one to get "save link target as" to work right)
Comment on attachment 276658 [details] [diff] [review]
v1.1 everything

I'm unbitrotting and addressing biesi's comments (as well as a few optimizations)
Attachment #276658 - Flags: review?(mano)
Attached patch v1.2 everything (obsolete) — Splinter Review
Attachment #276658 - Attachment is obsolete: true
Attachment #277132 - Flags: review?(mano)
Attached patch v1.3 everything (obsolete) — Splinter Review
Fixes comments on irc.
Attachment #277132 - Attachment is obsolete: true
Attachment #277156 - Flags: review?(mano)
Attachment #277132 - Flags: review?(mano)
Attached patch v1.4 (obsolete) — Splinter Review
Attachment #277156 - Attachment is obsolete: true
Attachment #277160 - Flags: review?(mano)
Attachment #277156 - Flags: review?(mano)
Comment on attachment 277160 [details] [diff] [review]
v1.4

+  var uir;

init that to beltzner.

+  if (aDownload.hasAttribute("referrer"))
+    uri = aDownload.getAttribute("referrer");

r=mano otherwise.
Attachment #277160 - Flags: review?(mano) → review+
For checkin
Attachment #277160 - Attachment is obsolete: true
Not sure how both of us missed this :(

+      if (!mReferrer) {
         nsCOMPtr<nsIHttpChannel> chan = do_QueryInterface(aRequest);
         if (chan) {
           rv = chan->GetReferrer(getter_AddRefs(mReferrer));
           if (NS_FAILED(rv) )
             mReferrer = nsnull;
         }
+      }
Checking in toolkit/components/downloads/public/Makefile.in;
new revision: 1.6; previous revision: 1.5
Checking in toolkit/components/downloads/public/nsIDownload.idl;
new revision: 1.20; previous revision: 1.19
Checking in toolkit/components/downloads/src/nsDownloadManager.cpp;
new revision: 1.102; previous revision: 1.101
Checking in toolkit/components/downloads/src/nsDownloadManager.h;
new revision: 1.34; previous revision: 1.33
Checking in toolkit/components/downloads/test/schema_migration/test_migration_to_2.js;
new revision: 1.2; previous revision: 1.1
Checking in toolkit/components/downloads/test/schema_migration/test_migration_to_3.js;
initial revision: 1.1
Checking in toolkit/components/downloads/test/schema_migration/v2.sqlite;
initial revision: 1.1
Checking in toolkit/mozapps/downloads/content/DownloadProgressListener.js;
new revision: 1.23; previous revision: 1.22
Checking in toolkit/mozapps/downloads/content/downloads.js;
new revision: 1.81; previous revision: 1.80
Checking in toolkit/mozapps/downloads/content/downloads.xul;
new revision: 1.28; previous revision: 1.27
Checking in embedding/components/ui/progressDlg/Makefile.in;
new revision: 1.6; previous revision: 1.5
Removing embedding/components/ui/progressDlg/nsIProgressDialog.idl;
new revision: delete; previous revision: 1.15
Checking in uriloader/base/Makefile.in;
new revision: 1.40; previous revision: 1.39
Removing uriloader/base/nsIDownload.idl;
new revision: delete; previous revision: 1.19
Checking in xpfe/components/download-manager/public/Makefile.in;
new revision: 1.4; previous revision: 1.3
Checking in xpfe/components/download-manager/public/nsIDownload.idl;
new revision: 1.3; previous revision: 1.2
Checking in widget/src/windows/nsDragService.cpp;
new revision: 1.56; previous revision: 1.55
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite-
Flags: in-litmus?
Resolution: --- → FIXED
Blocks: 392679
http://litmus.mozilla.org/show_test.cgi?id=4568

in-litmus+

Only the implementation is verified, not the functionality.  I'm going to spin off a separate bug, because, according to the design docs at http://wiki.mozilla.org/User:Dmose:Fx-Docs:DownloadManager/User_Interface, 

"clicking the URL icon will open a browser on the URL, which is the URI up to the point where the file name started".

Currently, it does nothing, and lists the file, which it shouldn't (just the path URI).

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a8pre) Gecko/2007081705 Minefield/3.0a8pre
Status: RESOLVED → VERIFIED
I spun off bug 392697.
Flags: in-litmus? → in-litmus+
Shouldn't "referrer" be added to the columns that must be present when downgrading the database schema, so that we rebuild the table if we downgrade from a future version that doesn't contain the referrer column?
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp&rev=1.106&mark=309#299
(In reply to comment #19)
> Shouldn't "referrer" be added to the columns that must be present when
> downgrading the database schema, so that we rebuild the table if we downgrade
> from a future version that doesn't contain the referrer column?
> http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp&rev=1.106&mark=309#299
Yes, good catch.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Attached patch v1.5.1Splinter Review
Thanks for catching that!
Attachment #277928 - Flags: review?(mano)
Attachment #277168 - Attachment description: v1.5 → v1.5 (checked in)
Keywords: checkin-needed
Checking in toolkit/components/downloads/src/nsDownloadManager.cpp;
new revision: 1.108; previous revision: 1.107
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Duplicate of this bug: 433323
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.