Now that we've added PasteboardCocoa.mm in https://trac.webkit.org/r222656, we can share more code between iOS and macOS. Also, we remove the layering violation inadvertently introduced in https://trac.webkit.org/r222595 whereby which Pasteboard code depends on Settings.
Created attachment 322277 [details] Cleanup
Created attachment 322279 [details] Cleanup
Comment on attachment 322279 [details] Cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=322279&action=review > Source/WebCore/dom/DataTransfer.cpp:146 > + if (Settings::customPasteboardDataEnabled() && !Pasteboard::isSafeTypeForDOMToReadAndWrite(type)) Shouldn't this be !Pasteboard::isSafeTypeForDOMToReadAndWrite(normalizedType) here? (instead of the non-normalized type)
(In reply to Wenson Hsieh from comment #3) > Comment on attachment 322279 [details] > Cleanup > > View in context: > https://bugs.webkit.org/attachment.cgi?id=322279&action=review > > > Source/WebCore/dom/DataTransfer.cpp:146 > > + if (Settings::customPasteboardDataEnabled() && !Pasteboard::isSafeTypeForDOMToReadAndWrite(type)) > > Shouldn't this be > !Pasteboard::isSafeTypeForDOMToReadAndWrite(normalizedType) here? (instead > of the non-normalized type) Let's augment editing/pasteboard/data-transfer-get-data-on-drop-custom.html so that instead of writing "text/plain", we try to write "Text" so that we have test coverage for this codepath where type normalization matters.
(In reply to Wenson Hsieh from comment #4) > (In reply to Wenson Hsieh from comment #3) > > Comment on attachment 322279 [details] > > Cleanup > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=322279&action=review > > > > > Source/WebCore/dom/DataTransfer.cpp:146 > > > + if (Settings::customPasteboardDataEnabled() && !Pasteboard::isSafeTypeForDOMToReadAndWrite(type)) > > > > Shouldn't this be > > !Pasteboard::isSafeTypeForDOMToReadAndWrite(normalizedType) here? (instead > > of the non-normalized type) > > Let's augment editing/pasteboard/data-transfer-get-data-on-drop-custom.html > so that instead of writing "text/plain", we try to write "Text" so that we > have test coverage for this codepath where type normalization matters. That's not going to quite work because this is a bug in getData, not setData. We need to write a new test for this.
Created attachment 322288 [details] Patch for landing
Comment on attachment 322288 [details] Patch for landing Wait for EWS.
(In reply to Ryosuke Niwa from comment #5) > (In reply to Wenson Hsieh from comment #4) > > (In reply to Wenson Hsieh from comment #3) > > > Comment on attachment 322279 [details] > > > Cleanup > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=322279&action=review > > > > > > > Source/WebCore/dom/DataTransfer.cpp:146 > > > > + if (Settings::customPasteboardDataEnabled() && !Pasteboard::isSafeTypeForDOMToReadAndWrite(type)) > > > > > > Shouldn't this be > > > !Pasteboard::isSafeTypeForDOMToReadAndWrite(normalizedType) here? (instead > > > of the non-normalized type) > > > > Let's augment editing/pasteboard/data-transfer-get-data-on-drop-custom.html > > so that instead of writing "text/plain", we try to write "Text" so that we > > have test coverage for this codepath where type normalization matters. > > That's not going to quite work because this is a bug in getData, not > setData. We need to write a new test for this. Oh, true. I suppose the new test could exercise both setData/getData with non-normalized versions of text/plain, text/html and text/uri-list and make sure that we can write and read these types from DOM (and also use internals.settings.setCustomPasteboardDataEnabled(true)). I can add this in a followup.
(In reply to Wenson Hsieh from comment #8) > > I suppose the new test could exercise both setData/getData with > non-normalized versions of text/plain, text/html and text/uri-list and make > sure that we can write and read these types from DOM (and also use > internals.settings.setCustomPasteboardDataEnabled(true)). I can add this in > a followup. Sure, feel free to write a test if you'd like. If not, I can write one as well.
Created attachment 322289 [details] Patch for landing
Comment on attachment 322289 [details] Patch for landing Wait for EWS (after WPE builds).
Created attachment 322290 [details] Patch for landing
Comment on attachment 322290 [details] Patch for landing Wait for WPE EWS again...
Created attachment 322293 [details] Fixed Mac builds
Comment on attachment 322293 [details] Fixed Mac builds Clearing flags on attachment: 322293 Committed r222680: <http://trac.webkit.org/changeset/222680>
All reviewed patches have been landed. Closing bug.
<rdar://problem/34754897>