Bug 117683 – Move Clipboard::declareAndWriteDragImage to DragController

• Component: HTML Editing
• Status: RESOLVED FIXED

[Darin Adler]

Move Clipboard::declareAndWriteDragImage to DragController

[Darin Adler]

Created attachment 204790 [details]
Patch

[Darin Adler]

Relatively straightforward refactoring, helps trim down the size of the Clipboard class and give it focus.

[Early Warning System Bot]

Comment on attachment 204790 [details]
Patch

Attachment 204790 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/930174

[Early Warning System Bot]

Comment on attachment 204790 [details]
Patch

Attachment 204790 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/867597

[Darin Adler]

Created attachment 204791 [details]
Patch

[Early Warning System Bot]

Comment on attachment 204791 [details]
Patch

Attachment 204791 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/867601

[Early Warning System Bot]

Comment on attachment 204791 [details]
Patch

Attachment 204791 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/904217

[Darin Adler]

Created attachment 204792 [details]
Patch

[Early Warning System Bot]

Comment on attachment 204792 [details]
Patch

Attachment 204792 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/944059

[Early Warning System Bot]

Comment on attachment 204792 [details]
Patch

Attachment 204792 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/928216

[Build Bot]

Comment on attachment 204792 [details]
Patch

Attachment 204792 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/943005

[Darin Adler]

Created attachment 205272 [details]
Patch

[Build Bot]

Comment on attachment 205272 [details]
Patch

Attachment 205272 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/964575

[Ryosuke Niwa]

Comment on attachment 205272 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=205272&action=review

> Source/WebCore/page/DragController.cpp:705
> +    if (element->isContentRichlyEditable()) {

Maybe it would be cleaner to move this check in the caller so that this function will just be named selectElement?

[Ryosuke Niwa]

Brent or Roger should look at the Windows port's change.

[Brent Fulgham]

Comment on attachment 205272 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=205272&action=review

I think the changes look good, but I'm curious about the original handling of the source URL; are we changing behavior in some unintentional fashion?

> Source/WebCore/platform/win/ClipboardWin.cpp:-69
> -    AtomicString imageURL = element->getAttribute(HTMLNames::srcAttr);

Do we not need this anymore? I wonder why this more complicated mechanism for retrieving the image source was used previously?

[Darin Adler]

Comment on attachment 205272 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=205272&action=review

>> Source/WebCore/platform/win/ClipboardWin.cpp:-69
>> -    AtomicString imageURL = element->getAttribute(HTMLNames::srcAttr);
> 
> Do we not need this anymore? I wonder why this more complicated mechanism for retrieving the image source was used previously?

I believe that this code check to see if there is any image in the element is redundant, and done at another level.

To be clear, the code is not retrieving the image source. Note below that fullURL is constructed so it can be checked to see if it’s empty. Nothing else is done with it.

[Brent Fulgham]

Comment on attachment 205272 [details]
Patch

r=me

[Darin Adler]

Committed r153978: <http://trac.webkit.org/changeset/153978>

[Ryosuke Niwa]

Windows build fixes landed in:
http://trac.webkit.org/changeset/153983
http://trac.webkit.org/changeset/153984
http://trac.webkit.org/changeset/153985
http://trac.webkit.org/changeset/153988

[Darin Adler]

Thanks for fixing Windows. Not sure why I didn’t notice the EWS failure!