Closed Bug 819378 Opened 13 years ago Closed 12 years ago

Entries kept in cache when using Download Helper add-on with youtube

Categories

(Firefox :: Extension Compatibility, defect)

Product:
Firefox
Component:
Extension Compatibility
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: virgil.dicu, Unassigned)

References

Details

(Whiteboard: [testday-20121207])

Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20121207 Firefox/20.0 1. Install Video Download Helper: https://addons.mozilla.org/en-US/firefox/addon/video-downloadhelper/?src=hp-dl-mostpopular 2. Clear cache. 3. Start Private Browsing 4. Visit youtube and download a video using download helper 5. Exit private Browsing Actual: private browsing data preserved (2 entries - one for the downloaded item) Expected: no data preserved
CCing the developer. Please note that this problem was noted with an experimental per-window private browsing build; no testing has occurred with a nightly build yet, but I would expect the same problems to occur.
I can confirm that it also occurs in Nightly. Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20121207 Firefox/20.0
Whiteboard: [testday-20121207]
I confirm the issue: when on youtube, Video DownloadHelper does not retain the original window when building the list of videos to be downloaded. So when the user requests the actual download, there is nothing to get the nsILoadContext from and null is passed as the last argument to nsIWebBrowserPersist.saveURI. This issue shouldn't be present when downloading videos from other sites than YouTube, since the proper load context is being used. The problem may also be present when downloading a gallery of images. From which Firefox version is the per-window private browsing enabled ?
(In reply to comment #3) > I confirm the issue: when on youtube, Video DownloadHelper does not retain the > original window when building the list of videos to be downloaded. So when the > user requests the actual download, there is nothing to get the nsILoadContext > from and null is passed as the last argument to nsIWebBrowserPersist.saveURI. > > This issue shouldn't be present when downloading videos from other sites than > YouTube, since the proper load context is being used. > > The problem may also be present when downloading a gallery of images. > > From which Firefox version is the per-window private browsing enabled ? It will hopefully be enabled in Firefox 20. Are you planning to use version detection to handle this?
Keep in mind that bug 794602 introduced the relevant changes, and that's on Beta right now.
(In reply to comment #5) > Keep in mind that bug 794602 introduced the relevant changes, and that's on > Beta right now. (And Beta is Firefox 18.)
The latest 4.9.12 version of Video DownloadHelper already uses with the extra nsILoadContext parameter of nsIWebBrowserPersist.saveURI when required (Firefox >=18). If you guys think having the YouTube + image gallery privacy-safe fix is urgent, i can roll out a 4.9.13 update next week. Otherwise, since the window-based privacy is only in Firefox 20, we can wait a few more weeks so i can have more features/fixes in the next add-on update.
Given your comment 3, I would expect to see the same problem present on FF 18.
Let me know if i'm wrong with my understanding of the situation: - Firefox 18 introduces a new nsILoadContext parameter to method saveURI of interface nsIWebBrowserPersist - Video DownloadHelper 4.9.12 handles this extra parameter, but in the case of YouTube (and possibly gallery download) it wrongly uses NULL as the parameter value - Firefox 20 introduces window-based privacy and makes use of the nsILoadContext to decide whether the download history must be kept or discarded The question is: will using NULL as nsILoadContext param in the call to nsIWebBrowserPersist.saveURI affect the expected behavior of Firefox 18 ? If so, i agree, we should release a new update of the addon.
When the parameter was introduced, it took effect. So yes, a wrong parameter in FF 18 will cause incorrect behaviour.
In particular, windows in versions of FF down to 10 or something have concepts of privacy. We're only exposing the actual UI to support multiple different types simultaneously in 20; under the hood more and more code is depending on the status of these windows.
Thanks for the precisions. So it sounds like we need a new add-on update. I'll post a link to the beta here next week so we can agree it solves the problem before pushing the new version to public.
Back to this issue. I just installed the nightly but i can't find a way to start a private window. From what i understood, it should be available from the browser File menu, right ? What am i missing ? I could verify that the "regular" private browsing mode works ok with VDH: switch to "full private", download a youtube video, exit the private mode, the Downloads dialog does not show the downloaded entry anymore.
It's not actually available on nightly yet; that should be happening today.
Still not there this morning. Please, let me know in this bug when the window private mode is available so i can make VDH comply with the specs.
It's now live in Nightly. You can grab the latest build here: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/
I've made the changes in this add-on version: http://www.downloadhelper.net/install-beta.php?version=4.9.13b1 In fact the youtube-specific capture method was properly using the load context but not the common method. I presume the original test on YouTube was performed with the common method (i.e. picking in the menu the entry with a red back arrow, not the youtube logo). The gallery capture method has also been fixed. Here is an URL where you can download a few images using this method: http://www.noao.edu/image_gallery/html/im1126.html Let me know if it's ok for you so i can roll-out an update.
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20121217 Firefox/20.0 I can still see an entry while simply visiting youtube in PB mode with the beta version of download helper. Latest Nightly with Ubuntu 12.04 1. Start Firefox Nightly with clean profile. 2. Install beta version of Download Helper. 3. Clear cache (all entries should now be 0 for about:cache) 4. Open a new private window. 5. Visit youtube. 6. Open a random video. 7. Close the private window. 8. Open about:cache in a normal browsing window. Entry in about:cache: http://www.youtube.com/get_video_info?video_id=KVu3gS7iJu4&fmt=18
The latest fix only relates to the entries in the download history. As long as the DownloadHelper icon is animating (by default, for 60 seconds after the start of the video), i presume it is normal to still see the entry in the cache. Is there any documentation about the new private mode implementation, like associated listeners or at least getting the privacy status of a given window ?
No specific page about that; it's so far scattered across doc pages about APIs that require privacy-related information. I'll try to write that up today, but you'll want the PrivateBrowsingUtils.jsm module: http://mxr.mozilla.org/mozilla-central/source/toolkit/content/PrivateBrowsingUtils.jsm
Thanks for the doc. Since in an addon has to deal with browser versions that either support per-window private browsing or not, what do you recommend as the best way to determine if loading PrivateBrowsingUtils.jsm and caring about this stuff is required ? Checking Firefox version ? Try to load PrivateBrowsingUtils.jsm and see if it generates an exception ? Checking the existence of a particular component or interface ? Any other way ?
It's recommended that addons use PrivateBrowsingUtils if it's available, and the global service if it's not. The relevant observer notifications and the jsm have been designed to maximize forward compatibility, such that they work in versions of Firefox <20 in conjunction with the global private browsing mode (ie. the last-pb-context-exited notification fires when leaving private browsing mode, and isWindowPrivate returns true when global private browsing mode is enabled).
Here is a new version of the addon: http://www.downloadhelper.net/install-beta.php?version=4.9.13b2 In addition of taking care of using the load context (when available) for starting the download, this version also clears the entries initiated from a private window as soon as a privacy context is exited. This seems to work fine from Firefox 3.0 (the first supported browser version) to the nightly. Let me know if it's ok for you.
Keywords: qawanted
Ehsan, what is your request of QA here?
(In reply to comment #25) > Ehsan, what is your request of QA here? To retest the latest version of the add-on and see if the problem has been fixed.
(In reply to :Ehsan Akhgari from comment #26) > (In reply to comment #25) > > Ehsan, what is your request of QA here? > > To retest the latest version of the add-on and see if the problem has been > fixed. Thank you. Virgil, can you please retest the latest version of the Download Helper add-on to confirm the problem you reported is fixed?
QA Contact: virgil.dicu
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20130106 Firefox/20.0 Used the version from comment 24 with the latest Nightly. The only entry I see in about:cache after using youtube and downloading videos is the youtube get info entry, which I assume is ok, judging by comment 19. No other entries in cache or Download history.
Keywords: qawanted
Well, i'm not completely happy with the fact the http://www.youtube.com/get_video_info?xxx entry remains in the cache. Now that all references to the download entry has been removed, i would have expected the Firefox code for managing the privacy stuff to clear this entry when exiting the private window. Note that the addon loads the get_video_info in background through an nsIChannel component, so it is not a regular window loading. Shouldn't Firefox mark the network resources associated to this channel as "to be removed when exiting privacy" ?
Thinking more about this, i don't see any way Firefox would know automatically that the requested nsIChannel has a privacy context. So i presume i must attach explicitly the privacy context to the channel. I haven't found in the doc how to do so. Any tips ? Here is the portion of code for loading the get_video_info url: var ioService = Components.classes["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService); var uri = ioService.newURI("http://www.youtube.com/get_video_info?video_id="+videoId+"&fmt=18", null, null); var channel = ioService.newChannelFromURI(uri); channel.asyncOpen(new StreamListener(...), null);
There are a few options. You can: * set the loadgroup of the channel to that of a relevant document * set the notification callbacks of the channel to an object that has a getInterface that can return a relevant nsILoadContext * QI to nsIPrivateBrowsingChannel and call setPrivate with an explicit privacy status boolean argument
Excellent ! Many thanks. I picked the third option and it did the trick: i don't get the get_video_info cache entry any more after exiting the private window. The new addon version is available at http://www.downloadhelper.net/install-beta.php?version=4.9.13b4 Virgil, when you will have confirmed it fixes the problem on your side, i will roll out the update.
All good for me with the last version of the add-on! No entries in cache or download.
Marking resolved fixed based on comment 33.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Version 4.9.13 of Video DownloadHelper has been submitted for review to amo
Virgil, can you please verify this is fixed with the AMO version?
Keywords: verifyme
Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20130111 Firefox/21.0 All good with Firefox 21 and add-on version 4.9.13. Thanks, Michel.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.