Last Comment Bug 308967 - dropping an image into an editor inserts a link
: dropping an image into an editor inserts a link
Status: RESOLVED FIXED
: fixed1.8
Product: Core
Classification: Components
Component: Drag and Drop (show other bugs)
: Trunk
: x86 OS/2
: -- normal (vote)
: ---
Assigned To: Mike Kaply [:mkaply]
:
: Neil Deakin
Mentors:
Depends on:
Blocks: 314605
  Show dependency treegraph
 
Reported: 2005-09-17 10:16 PDT by Rich Walsh
Modified: 2005-11-09 08:17 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
revises native d&d image handling (2.13 KB, patch)
2005-09-17 10:21 PDT, Rich Walsh
mozilla: review+
Details | Diff | Splinter Review
Patch with "nicer" string handling (2.11 KB, patch)
2005-10-31 10:05 PST, Peter Weilbacher
mozilla: review+
mtschrep: approval1.8rc2+
Details | Diff | Splinter Review

Description Rich Walsh 2005-09-17 10:16:34 PDT
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 2005-09-17 10:21:06 PDT
Created attachment 196431 [details] [diff] [review]
revises native d&d image handling
Comment 2 Mike Kaply [:mkaply] 2005-09-19 11:10:03 PDT
Just out of curiosity, is this how Windows handles this issue?
Comment 3 Peter Weilbacher 2005-09-19 11:22:10 PDT
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 2005-09-19 12:53:31 PDT
(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 2005-09-19 13:01:09 PDT
(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 2005-09-20 20:13:01 PDT
(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 2005-09-20 20:16:46 PDT
> > 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 2005-09-24 05:55:22 PDT
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] 2005-10-13 09:00:56 PDT
Comment on attachment 196431 [details] [diff] [review]
revises native d&d image handling

r=mkaply
Comment 10 Peter Weilbacher 2005-10-31 10:05:01 PST
Created attachment 201437 [details] [diff] [review]
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 2005-10-31 21:42:14 PST
(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] 2005-11-04 07:15:01 PST
Comment on attachment 201437 [details] [diff] [review]
Patch with "nicer" string handling

OS/2 only fix
Comment 13 Mike Schroepfer 2005-11-04 12:03:56 PST
Comment on attachment 201437 [details] [diff] [review]
Patch with "nicer" string handling

OS/2 Only - approved to land

Note You need to log in before you can comment on or make changes to this bug.