Bug 131844 - REGRESSION (r155700): Pasting an image into content-editable regions is broken
Summary: REGRESSION (r155700): Pasting an image into content-editable regions is broken
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-04-18 09:37 PDT by Brady Eidson
Modified: 2014-06-04 03:29 PDT (History)
2 users (show)

See Also:


Attachments
Fix without test (1.15 KB, patch)
2014-04-18 09:41 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Fix with attempted test. (11.54 KB, patch)
2014-04-18 13:15 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2014-04-18 09:37:50 PDT
REGRESSION (r155700): Pasting an image into content-editable regions is broken

1. Create a page with a content editable region
2. Open an image in Preview.app
3. Copy that image onto your pasteboard (so your pasteboard has *only* an image type on it)
4. Select-all in the content editable region, then paste.

You see the broken image icon instead of the image you pasted.

In radar <rdar://problem/16654156>
Comment 1 Brady Eidson 2014-04-18 09:40:39 PDT
This happens because 155700 reorders the "get document fragment from image resource" code to add the image resource to the DocumentLoader *after* creating the document fragment that uses it.

Previously we added it before.

Attaching the small patch that fixes the bug by restoring the previous behavior.

Seems like this can be layout testable, but I don't have the cycles to follow through with the test today.
Comment 2 Brady Eidson 2014-04-18 09:41:03 PDT
Created attachment 229648 [details]
Fix without test
Comment 3 Brady Eidson 2014-04-18 13:14:40 PDT
I've messed around trying to get a test for this for about 3 hours so I could land it.  The test obviously works in the browser.  But no matter what I do I can't get it to work in TestWebKitAPI.

I literally can't spend anymore time on this right now.  It'd be awesome if somebody could figure it out to land.
Comment 4 Brady Eidson 2014-04-18 13:15:27 PDT
Created attachment 229669 [details]
Fix with attempted test.
Comment 5 Brady Eidson 2014-04-18 16:27:10 PDT
We should land this without the test for now.  The fix is restoring old working behavior, and this bug is blocking more important work.

r? on the first patch.
Comment 6 Enrica Casucci 2014-04-18 16:28:20 PDT
Please add the ChangeLog
Comment 7 Brady Eidson 2014-04-18 16:29:29 PDT
Of course.
Comment 8 Brady Eidson 2014-04-18 16:31:02 PDT
Landed the fix itself in https://trac.webkit.org/changeset/167517

Will leave this bug open for future exploration into getting the test working.
Comment 9 Csaba Osztrogonác 2014-06-04 03:29:03 PDT
Comment on attachment 229648 [details]
Fix without test

Cleared Enrica Casucci's review+ from obsolete attachment 229648 [details] so that this bug does not appear in http://webkit.org/pending-commit.