308967 - dropping an image into an editor inserts a link

Status: RESOLVED FIXED

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

Description

[Rich Walsh]

When users drop an image file or link into an HTML editor window, they expect
that the image will be inserted.  Instead, they get a link.  The reason is that
the HTML flavor created by nsDragService is always formatted with an anchor tag.

This patch modifies the HTML creation code to look for extensions GIF, PNG, JPG,
and JPEG.  If found, it uses an IMG tag and sets the 'Alt' text to the URL's
title (if any).

FWIW... it would be really nice if this patch, along with Bug #243270, could
make it into the 1.8 branch.  Together, they would give the OS/2 versions of
Thunderbird 1.5 & Seamonkey 1.0 features that other platforms have had for years.

Comment 1

[Rich Walsh]

Attachment: revises native d&d image handling

Comment 2

[Mike Kaply [:mkaply]]

Just out of curiosity, is this how Windows handles this issue?

Comment 3

[Peter Weilbacher]

I tried this in NVU and it seems to work mostly. In some cases it inserts
strange image source URIs, like
 <img src="../../../m:/Graphics/Tests/anim.gif"
while the correct entry would be something like
 <img src="../../Graphics/Tests/anim.gif"
or just
 <img src="m:/Graphics/Tests/anim.gif"


But, should there not be a way to ask Mozilla for the supported image
extensions? In my MNG builds I would have to add at least two more lines like
                !stricmp(extension.get(), "mng") ||
                !stricmp(extension.get(), "jng") ||
while I am pretty sure that Mozilla keeps the list of supported image type
somewhere already. Otherwise, another one to add for official builds would be BMP.

Ah, there is imgLoader::GetMimeTypeFromContent in
modules\libpr0n\src\imgLoader.cpp that returns NS_ERROR_NOT_AVAILABLE if a
non-supported image type is found and there is some factory stuff (that I never
understood) in modules\libpr0n\build\nsImageModule.cpp that might help as well.

Comment 4

[Rich Walsh]

(In reply to comment #2)
> Just out of curiosity, is this how Windows handles this issue?

Probably not.  IIUC, COM-originated drags provide flavor-data so they don't have
to think about that aspect.  They may still have to compose html - I don't know
but I'll investigate.

Comment 5

[Rich Walsh]

(In reply to comment #3)
> I tried this in NVU and it seems to work mostly. In some cases it inserts
> strange image source URIs, like
>  <img src="../../../m:/Graphics/Tests/anim.gif"

Using a Thunderbird Mail compose window & Seamonkey's Composer, I haven't seen
these anomalies.  You may want to add some printf()s to NativeDragover() &
NativeDataToTransferrable() to watch the filename get changed into a URL & then
get formatted as HTML.

> But, should there not be a way to ask Mozilla for the supported image
> extensions? [snip] Ah, there is imgLoader::GetMimeTypeFromContent in
> modules\libpr0n\src\imgLoader.cpp that returns NS_ERROR_NOT_AVAILABLE if a
> non-supported image type is found

I knew this was less than desirable but I didn't know where to look.  I'll see
if the suggested method fits the bill.

Comment 6

[Rich Walsh]

(In reply to comment #3)
> But, should there not be a way to ask Mozilla for the supported image
> extensions?  Ah, there is imgLoader::GetMimeTypeFromContent in
> modules\libpr0n\src\imgLoader.cpp that returns NS_ERROR_NOT_AVAILABLE

This isn't suitable.  It's a sniffer that takes the first few bytes of the
file's content.  Using during d&d this for a file that may not be stored locally
is impractical at best.  If there were a listing of supported file extensions,
it would still be problematic because it would include .bmp & .ico.  Since only
Windows-format bitmaps & icons are supported, you'd have to use a sniffer to
distinguish between them & the unsupported OS/2 varieties.

> In my MNG builds I would have to add at least two more lines like
> !stricmp(extension.get(), "mng") || !stricmp(extension.get(), "jng")

Is there a commandline -define that identifies the presence of these extra
decoders?  If so, I can add an #ifdef.  Otherwise, I can add an "#if 0" that you
can change to #if 1"  :-)

Comment 7

[Rich Walsh]

> > Just out of curiosity, is this how Windows handles this issue?
>
> I don't know but I'll investigate.

It appears that COM supplies preformatted HTML so the Win code doesn't have to
deal with this at all.

Comment 8

[Peter Weilbacher]

OK, I didn't think about how unpractival sniffing is at that point. Another
thing I found was the nsModuleComponentInfo structure in
xpfe/components/build/nsModule.cpp, but that has similar problems because of the
non-uniqueness of the file extensions, and one probably doesn't have access to
that easily. (Btw, OS/2 BMPs are supported as well, but only the old
uncompressed version.)

And don't worry about the MNG lines. It's the job of the MNG patch to add
something like that. I will alert glennrp and opi once this got in.

Finally, perhaps another improvement that makes more sense: you might want to
use extension.LowerCaseEqualsLiteral("gif") instead of !stricmp(extension.get(),
"gif"). The XPCOM code in NVU is too old for that but it seems to be more
Mozilla-like and seems to work for nsCAutoString.

Comment 9

[Mike Kaply [:mkaply]]

Comment on attachment 196431 [details] [diff] [review]
revises native d&d image handling

r=mkaply

Comment 10

[Peter Weilbacher]

Attachment: Patch with "nicer" string handling

Same patch as Rich's, just with
   extension.LowerCaseEqualsLiteral("gif")
etc. instead of
   !stricmp(extension.get(), "gif")
etc.

Comment 11

[Rich Walsh]

(In reply to comment #10)
> Created an attachment (id=201437) [edit]
> Patch with "nicer" string handling

Sorry, I forgot about this one.  Thanks for fixing it.

Comment 12

[Mike Kaply [:mkaply]]

Comment on attachment 201437 [details] [diff] [review]
Patch with "nicer" string handling

OS/2 only fix

Comment 13

[Mike Schroepfer]

Comment on attachment 201437 [details] [diff] [review]
Patch with "nicer" string handling

OS/2 Only - approved to land