User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b) Gecko/20040401
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b) Gecko/20040401
I cannot download from "Save As..." button that is on "media" tab on Page Info
Window.
It can be reproduced with WinXP and Linux.
On Mac, media tab is not completed to initialize.
Reproducible: Always
Steps to Reproduce:
1. Select image or other object, but it is not type = background(See bug 178469).
2. Push "Save As..." button.
3. Choose file name.
Actual Results:
It is added to download manager or opened progless window.
But It is not started to download.
Expected Results:
It should be started to download, and should be completed.

I see this also on XP with the latest Moz nightly. Images characterized on the
media tab as "image" can download, but those known as "background" don't save:
the Save As button does nothing. Seems like this has now been confirmed several
times eh? ;-)

(In reply to comment #6)
> I see this also on XP with the latest Moz nightly. Images characterized on the
> media tab as "image" can download, but those known as "background" don't save:
> the Save As button does nothing.
That is Bug 178469.

Confirmed.
I get the images in the download manager but with the state "Starting..." all
the time and not being downloaded.
- Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a) Gecko/20040514
Firefox/0.8.0+
- Microsoft Windows 2000 Pro 5.00.2195 SP4

Mozilla 2004072808 WinNT4.0
Now I get the following error in JS console if I try a Save As in the media tab
for a normal image (appears after selecting save in the save as dialog):
Error: XPCNativeWrapper is not defined
Source File: chrome://communicator/content/contentAreaUtils.js
Line: 48

Created attachment 154948[details][diff][review]
patch
As proposed in comment #3, start of the try statement in getReferrer()
(from contentAreaUtils.js) moved up a bit so that the failing line is caught by
the catch and the function just returns null.
<script type="application/x-javascript"
src="chrome://communicator/content/XPCNativeWrapper.js"/>
added to pageInfo.xul to fix error in comment #12

I think the same problem caused bug 248964, and I have the same symptoms even
for downloading some files (ie., if I open directly an URL ending with a
filename to download in a new browser window, without navigating the site.

(In reply to comment #14)
> It's fixed the same way in bug 248964.Bug 248964 is now fixed and it works with Mozilla 2004081008 on WinNT4. But Save
As for Page Info/Media does still not work. But the error has changed.
Now I get the following error in JS console if I try a Save As in the media tab
for a normal image (appears after selecting save in the save as dialog). Note:
No download progress dialog appears (or it is too fast).
Error: [Exception... "Component returned failure code: 0x80040154
(NS_ERROR_FACTORY_NOT_REGISTERED) [nsIDownload.init]" nsresult: "0x80040154
(NS_ERROR_FACTORY_NOT_REGISTERED)" location: "JS frame ::
chrome://communicator/content/contentAreaUtils.js :: foundHeaderInfo :: line
404" data: no]
Source File: chrome://communicator/content/contentAreaUtils.js
Line: 404

(In reply to comment #17)
> But the error has changed.
Build 2004081109 on WinNT4 changed back again, amazing. Now it is the same error
like in comment 12. The Saving dialog pops up but never finishes or saves something.

I need to trace through all of this, since I don't really know how it works. I'm
guessing, but it's probably trying to use the page info window as the window to
get the referring url from, which is wrong. Or something. At any rate, this will
have to wait since I'm stuck at my parents place and my Dad just volunteered me
to help put plywood over the windows. :P I hate hurricanes.

Oh, and it has to send the correct referrer, because there's a certain commonly
accessed class of websites (porn) that check the referrer you send to see if you
are allowed to view the images that page contains, even though this isn't a very
robust authorization method.

(In reply to comment #25)
> What's the difference in the patch here and the patch for bug 178469?
> Looks like this one includes that patch and then some extra changes.
As Florian stated this fixes bug 178469 and images embedded with the OBJECT tag
(bug 235447). Since this is a more complete fix we probably want this over the
patch in 178469.

Comment on attachment 157758[details][diff][review]
a better patch
Since use of referrer depends on url not being null (and if url != null, then
item != null holds), either move the var referrer inside the |if (url)| without
the |if (item)| or apply the "avoid one time vars" rule and inline it, so:
if (url)
saveURL(url, null, 'SaveImageTitle', false, makeURL(item.baseURI));
(yes, I removed that silly space before ')', per the style guide)
- var referer = getReferrer(document);
+ var referer = aData.referrer || getReferrer(document);
So why is that using |document| instead of |aData.document|? Would that solve
our problem instead of passing in the referrer explicitely?

Created attachment 161256[details][diff][review]
patch
(In reply to comment #32)
> (From update of attachment 157758[details][diff][review])
> Since use of referrer depends on url not being null (and if url != null, then> item != null holds), either move the var referrer inside the |if (url)|
without
> the |if (item)| or apply the "avoid one time vars" rule and inline it, so:
>
Yes, item can't be null if url isn't.
> if (url)
> saveURL(url, null, 'SaveImageTitle', false, makeURL(item.baseURI));
>
> (yes, I removed that silly space before ')', per the style guide)
I prefer this.
> - var referer = getReferrer(document);
> + var referer = aData.referrer || getReferrer(document);
>
>
> So why is that using |document| instead of |aData.document|? Would that solve> our problem instead of passing in the referrer explicitely?
>
saveInternal is sometimes called with null for aDocument, for example by
saveDocument or SaveURL (SaveURL isn't only called by page info).
So we can't use getReferrer(aData.document) because when aData.document is
null, getReferrer yields the error "TypeError: doc has no properties".
Moreover, passing in the referrer explicitely is faster: getReferrer calls
getContentFrameURI which calls isContentFrame and XPCNativeWrapper.
The new version of the patch includes the fix for both Seamonkey and Firefox in
one patch.
It uses "saveURL(url, null, 'SaveImageTitle', false, makeURL(item.baseURI));".

Comment on attachment 161256[details][diff][review]
patch
Ben, if the previous patch isn't checked in Aviary yet, i prefer this new one,
it is shorter. The only change is
if (url)
saveURL(url, null, 'SaveImageTitle', false, makeURL(item.baseURI));
instead of
if (item)
var referrer = makeURL(item.baseURI);
if (url)
saveURL(url, null, 'SaveImageTitle', false, false, referrer);

Comment on attachment 161256[details][diff][review]
patch
Florian: a recent checkin to contentAreaUtils.js makes this patch no longer
apply. I don't think it's hard to update.
Otherwise it looks good. I buy your argument on passing in aReferrer
explicitely. Are there other places that could do that, taking the guess-work
out of it?
Also, if aData.document can be null, how about var referrer = aData.referrer ||
getReferrer(aData.document || document)?
Though it seems in the new code |aData| is gone. Ah, but there's |aDocument|
instead. Anyway, just some ponderings.
sr=jag, but marking obsolete to indicate it needs updating.

(In reply to comment #40)
> Also, if aData.document can be null, how about var referrer = aData.referrer ||
> getReferrer(aData.document || document)?
>
0001: aDocument (it's the same type for aData.document)
$[0] = [HTMLDocument] [class: HTMLDocument] {2}
0001: document
$[1] = [XULDocument] [class: XULDocument] {1}
(i pasted this from Venkman)
getReferrer expects a parameter of type "XULDocument", so we can't use
getReferrer(aData.document || document).
I've tried to use:
var referrer = aDocument ? makeURI(aDocument.URL) : getReferrer(document)
We can get an [HTMLdocument] value from page info with:
imageView.data[tree.currentIndex][3].ownerDocument
But passing this value to internalSave as aDocument doesn't work because
internalSave uses this value to find the content type of the file to save, it
detects text/html type and then fails in "getPostData()" :
"Error ``getWebNavigation is not defined'' [xs] in file
``chrome://communicator/content/contentAreaUtils.js'', line 585, character 0.".
So I think i'll have to pass the referrer explicitely in the updated patch, like
i did in previous patch.

Comment on attachment 162252[details][diff][review]
updated patch
Yeah, like I said, passing the referrer in explicitely seems the right way to
go.
I guess what bugs me is that saveInternal assumes that |document| is the
document containing the item you want to save. If that could be made more
explicit, e.g. |getReferrer(aContainingDocument)|, then that code would work.
Alternatively, the only place that refers to |document| is that getReferrer
call, so if that can be avoided by passing in the refferer explicitely in all
calls, life will be good too. Perhaps in another bug report though.
sr=jag

I have tested and this bug is still active in Moz 1.7.5, 1.7.3, 1.7.2 (and
probably more older versions).
Luckilly the fix from Comment 3 still works in these versions (moving the TRY
statement in the contentAreaUtils.js file).

Comment on attachment 162252[details][diff][review]
updated patch
requesting a= for 1.7 branch. not security related or anything special like
that, just really annoying to keep fielding bugs about this.