214626 - [FIX]<a> tag attrib type="image/jpeg" ignored when using href="javascript:foo()" where foo() returns contents of a binary jpeg file

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect, P1)

Description

[Shaun Crampton]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6 StumbleUpon/1.73
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6 StumbleUpon/1.73

when using <a type="image/jpeg" href="javascript:foo()"> where the javascript
function foo() returns the contents of a jpeg file the jpeg data is dumped as
text to the screen when I would expect the type="image/jpeg" attrib to direct
the browser to display it as a jpeg. 

The same bug is present if I embed a jpeg in a html file using <img
src="javascript:foo()"> if I then click 'View image' on the context menu I
recieve the same dump however the image is correctly displayed inline.

The application of this javascript is to create a self decrypting webpage (enter
a password and the page decrypts itself via a javascript des library) so all the
images must be inlined in this fashion.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Shaun Crampton]

Attachment: Test case

Page with embedded image and link to embedded image.produced via <img
src="javasript:foo()"> and <a type="image/jpeg" href="javasript:foo()">
respectively. On click view image on the former the data is dumper to the
screen as text and on clicking the latter the same is true despite the type
attribute.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Taking; I've been meaning to implement this anyway.  I'll do it when I get back.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 132260 [details] [diff] [review]
Proposed patch

Adam, what do you think?  Is it ok to make webshell d this, or should I add
"type" to the nsILinkHandler methods and pass it from content?

Also, would it make sense to add an advisory type param to loadURI()?  For now
I just added it to loadInternal....

Comment 5

[Adam Lock]

I think the patch looks okay in principle although session history would have to
be dealt with and the XXXbz comments would need to be fixed. Changing loadURI
would require a lot of changes all over including js in Moz / Firebird etc., so
perhaps we should consider leaving that for the moment.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch for review and all that

This fixes up session history and removes the XXX comments.  This should be
ready to go, I think....

Comment 7

[Adam Lock]

Comment on attachment 132321 [details] [diff] [review]
Patch for review and all that

r=adamlock

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 132321 [details] [diff] [review]
Patch for review and all that

darin, would you review?

Comment 9

[Darin Fisher]

Comment on attachment 132321 [details] [diff] [review]
Patch for review and all that

>Index: xpfe/components/shistory/public/nsISHEntry.idl

>+/** attribute to indicate the content-type of the document that this
>+    is a session history entry for **/
>+attribute ACString contentType; 

nit: all other comments seem to end with a single * before the /


> /** Additional ways to create an entry */
> void create(in nsIURI aURI, in wstring aTitle, in nsIDOMDocument aDocument,
>+            in nsIInputStream aInputStream, in nsILayoutHistoryState aHistoryLayoutState,
>+            in nsISupports aCacheKey, in ACString aContentType);

nit: AString'ify aTitle?  maybe save it for a later patch or can it
sometimes be null?


>Index: docshell/base/nsDocShell.cpp

>+    else {
>+        nsAutoString contentTypeHint;
>+        nsCOMPtr<nsIDOMWindow> window = do_GetInterface((nsIDocShell*)this);
>+        if (window) {
>+            nsCOMPtr<nsIDOMDocument> document;
>+            window->GetDocument(getter_AddRefs(document));
>+            nsCOMPtr<nsIDOMNSDocument> doc(do_QueryInterface(document));
>+            if (doc) {
>+                doc->GetContentType(contentTypeHint);
>+            }
>+        }

nit: be consistent in nsCOMPtr<T> initialization.  either use operator=()
or ctor style for all.


sr=darin

Comment 10

[Boris Zbarsky [:bzbarsky]]

> nit: all other comments

Yep.  Fixed.

> maybe save it for a later patch

I'd rather, since it's in fact null some places and I'd need to change the
SetTitle code too...

> nit: be consistent

Fixed.

Patch checked in.

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

in combination with bug 247607's patch, this caused bug 249107. <sigh>