Bug 175704

Summary: Rename DataTransferAccessPolicy to match spec and refactor related node
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Cleanup
none
Fixed iOS builds
none
Patch for landing
none
Patch for landing
none
Patch for landing none

Description Ryosuke Niwa 2017-08-17 20:04:44 PDT
Rename DataTransferAccessPolicy to match the drag data store mode:
https://html.spec.whatwg.org/multipage/dnd.html#drag-data-store-mode

Also refactor related code.
Comment 1 Ryosuke Niwa 2017-08-17 20:16:56 PDT
Created attachment 318462 [details]
Cleanup
Comment 2 Ryosuke Niwa 2017-08-17 20:41:40 PDT
Created attachment 318463 [details]
Fixed iOS builds
Comment 3 Wenson Hsieh 2017-08-17 21:19:22 PDT
Comment on attachment 318463 [details]
Fixed iOS builds

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

> Source/WebCore/editing/Editor.cpp:331
> +        storeMode = DataTransfer::StoreMode::Invalid;

Possibly helpful to ASSERT() that eventType is one of the other clipboard event types here, that is not handled above?

> Source/WebCore/editing/Editor.cpp:346
> +        pasteboard->writePasteboard(dataTransfer->pasteboard());

This reminds me -- we'll need to actually implement a lot of these methods on iOS. Pasteboard::writePasteboard, for example, is stubbed out in PasteboardIOS.mm

> Source/WebCore/page/DragController.cpp:233
> +    DragOperation srcOpMask = dragData.draggingSourceOperationMask();

I don't think this temporary variable adds much clarity.
Comment 4 Ryosuke Niwa 2017-08-17 21:28:48 PDT
(In reply to Wenson Hsieh from comment #3)
> Comment on attachment 318463 [details]
> Fixed iOS builds
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=318463&action=review
> 
> > Source/WebCore/editing/Editor.cpp:331
> > +        storeMode = DataTransfer::StoreMode::Invalid;
> 
> Possibly helpful to ASSERT() that eventType is one of the other clipboard
> event types here, that is not handled above?

Done that.

> > Source/WebCore/editing/Editor.cpp:346
> > +        pasteboard->writePasteboard(dataTransfer->pasteboard());
> 
> This reminds me -- we'll need to actually implement a lot of these methods
> on iOS. Pasteboard::writePasteboard, for example, is stubbed out in
> PasteboardIOS.mm

Oh, we'd most certainly need to do that. Does that mean we can't currently override contents of the clipboard from copy / cut events right now?

> > Source/WebCore/page/DragController.cpp:233
> > +    DragOperation srcOpMask = dragData.draggingSourceOperationMask();
> 
> I don't think this temporary variable adds much clarity.

Removed.
Comment 5 Ryosuke Niwa 2017-08-17 21:36:07 PDT
Created attachment 318470 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2017-08-17 22:06:52 PDT
Comment on attachment 318470 [details]
Patch for landing

Rejecting attachment 318470 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
64/ExtensionStyleSheets.dia -c /Volumes/Data/EWS/WebKit/Source/WebCore/dom/ExtensionStyleSheets.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/ExtensionStyleSheets.o

** BUILD FAILED **


The following build commands failed:
	CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/DragController.o page/DragController.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)

Full output: http://webkit-queues.webkit.org/results/4334776
Comment 7 Ryosuke Niwa 2017-08-17 22:33:45 PDT
Created attachment 318472 [details]
Patch for landing
Comment 8 Ryosuke Niwa 2017-08-17 22:50:21 PDT
Comment on attachment 318472 [details]
Patch for landing

Oops, I broke internal iOS builds.
Comment 9 Ryosuke Niwa 2017-08-17 22:52:20 PDT
Created attachment 318476 [details]
Patch for landing
Comment 10 Ryosuke Niwa 2017-08-17 22:52:38 PDT
Comment on attachment 318476 [details]
Patch for landing

Wait for EWS.
Comment 11 Ryosuke Niwa 2017-08-18 14:53:52 PDT
Comment on attachment 318476 [details]
Patch for landing

Clearing flags on attachment: 318476

Committed r220935: <http://trac.webkit.org/changeset/220935>
Comment 12 Ryosuke Niwa 2017-08-18 14:53:54 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Radar WebKit Bug Importer 2017-08-18 14:55:19 PDT
<rdar://problem/33970723>