Summary: | Move Clipboard::declareAndWriteDragImage to DragController | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||||||
Component: | HTML Editing | Assignee: | Darin Adler <darin> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bfulgham, cdumez, commit-queue, esprehn+autocc, gyuyoung.kim, kangil.han, philn, rakuco, rego+ews, rniwa, roger_fong, webkit-ews, xan.lopez | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Darin Adler
2013-06-16 11:33:52 PDT
Created attachment 204790 [details]
Patch
Relatively straightforward refactoring, helps trim down the size of the Clipboard class and give it focus. Comment on attachment 204790 [details] Patch Attachment 204790 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/930174 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 Created attachment 204791 [details]
Patch
Comment on attachment 204791 [details] Patch Attachment 204791 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/867601 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 Created attachment 204792 [details]
Patch
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 Comment on attachment 204792 [details] Patch Attachment 204792 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/928216 Comment on attachment 204792 [details] Patch Attachment 204792 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/943005 Created attachment 205272 [details]
Patch
Comment on attachment 205272 [details] Patch Attachment 205272 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/964575 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? Brent or Roger should look at the Windows port's change. 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? 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. Comment on attachment 205272 [details]
Patch
r=me
Committed r153978: <http://trac.webkit.org/changeset/153978> 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 Thanks for fixing Windows. Not sure why I didn’t notice the EWS failure! |