RESOLVED FIXED 190108
Add a new variant of serializePreservingVisualAppearance which takes VisibleSelection
https://bugs.webkit.org/show_bug.cgi?id=190108
Summary Add a new variant of serializePreservingVisualAppearance which takes VisibleS...
Ryosuke Niwa
Reported 2018-09-28 23:23:15 PDT
Add a new variant of serializePreservingVisualAppearanceInternal which takes VisibleSelection directly instead of Range. This fixes a bug exhibited in LayoutTests/editing/pasteboard/paste-table-003.html for example.
Attachments
Adds a variant and fixes a bug (14.95 KB, patch)
2018-09-28 23:35 PDT, Ryosuke Niwa
no flags
Updated for trunk (14.41 KB, patch)
2018-09-30 22:46 PDT, Ryosuke Niwa
no flags
Patch for landing (14.24 KB, patch)
2018-10-01 16:16 PDT, Ryosuke Niwa
ews-watchlist: commit-queue-
Archive of layout-test-results from ews104 for mac-sierra-wk2 (2.90 MB, application/zip)
2018-10-01 18:51 PDT, EWS Watchlist
no flags
Ryosuke Niwa
Comment 1 2018-09-28 23:34:48 PDT
I wanted to do this in a simple patch in the bug 190107 but this happens to also fixes a bug so I split it into its own patch.
Ryosuke Niwa
Comment 2 2018-09-28 23:35:02 PDT
Created attachment 351174 [details] Adds a variant and fixes a bug
Ryosuke Niwa
Comment 3 2018-09-28 23:35:40 PDT
Note that this patch depends on the patch posted on the bug 190107.
Ryosuke Niwa
Comment 4 2018-09-30 22:46:56 PDT
Created attachment 351226 [details] Updated for trunk
Wenson Hsieh
Comment 5 2018-10-01 07:18:59 PDT
Comment on attachment 351226 [details] Updated for trunk View in context: https://bugs.webkit.org/attachment.cgi?id=351226&action=review > Source/WebCore/platform/win/PasteboardWin.cpp:472 > + // FIXME: Use ResolveURLs::YesExcludingLocalFileURLsForPrivacy Nit - period at end of this comment. > Source/WebKit/WebProcess/WebCoreSupport/gtk/WebEditorClientGtk.cpp:125 > pasteboardContent.text = range->text(); It looks like this GTK code still grabs text from the selected Range — perhaps we could just keep the VisibleSelection => Range conversion here to call Range::text(), or add a variant of plainText() in TextIterator.h that takes a VisibleSelection instead of a Range and call that instead.
Ryosuke Niwa
Comment 6 2018-10-01 16:15:30 PDT
(In reply to Wenson Hsieh from comment #5) > Comment on attachment 351226 [details] > Updated for trunk > > View in context: > https://bugs.webkit.org/attachment.cgi?id=351226&action=review > > > Source/WebCore/platform/win/PasteboardWin.cpp:472 > > + // FIXME: Use ResolveURLs::YesExcludingLocalFileURLsForPrivacy > > Nit - period at end of this comment. Fixed. > > Source/WebKit/WebProcess/WebCoreSupport/gtk/WebEditorClientGtk.cpp:125 > > pasteboardContent.text = range->text(); > > It looks like this GTK code still grabs text from the selected Range — > perhaps we could just keep the VisibleSelection => Range conversion here to > call Range::text(), or add a variant of plainText() in TextIterator.h that > takes a VisibleSelection instead of a Range and call that instead. Yeah, I'm gonna keep the range conversion for now.
Ryosuke Niwa
Comment 7 2018-10-01 16:16:08 PDT
Created attachment 351320 [details] Patch for landing
Ryosuke Niwa
Comment 8 2018-10-01 16:16:29 PDT
Comment on attachment 351320 [details] Patch for landing Wait for EWS.
Radar WebKit Bug Importer
Comment 9 2018-10-01 16:21:01 PDT
EWS Watchlist
Comment 10 2018-10-01 18:50:58 PDT
Comment on attachment 351320 [details] Patch for landing Attachment 351320 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/9419897 New failing tests: media/range-extract-contents-crash.html
EWS Watchlist
Comment 11 2018-10-01 18:51:00 PDT
Created attachment 351334 [details] Archive of layout-test-results from ews104 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Ryosuke Niwa
Comment 12 2018-10-01 18:58:55 PDT
I don't think the failure in the media test is related to this change: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 ??? 000000000000000000 0 + 0 1 com.apple.WebKit 0x0000000105860775 WebKit::PlaybackSessionModelContext::externalPlaybackChanged(bool, WebCore::PlaybackSessionModel::ExternalPlaybackTargetType, WTF::String const&) + 185 (Vector.h:1504) 2 com.apple.WebKit 0x000000010570842e void IPC::callMemberFunctionImpl<WebKit::PlaybackSessionManagerProxy, void (WebKit::PlaybackSessionManagerProxy::*)(unsigned long long, bool, unsigned int, WTF::String), std::__1::tuple<unsigned long long, bool, unsigned int, WTF::String>, 0ul, 1ul, 2ul, 3ul>(WebKit::PlaybackSessionManagerProxy*, void (WebKit::PlaybackSessionManagerProxy::*)(unsigned long long, bool, unsigned int, WTF::String), std::__1::tuple<unsigned long long, bool, unsigned int, WTF::String>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul>) + 66 (utility:753) 3 com.apple.WebKit 0x00000001057073c1 void IPC::handleMessage<Messages::PlaybackSessionManagerProxy::ExternalPlaybackPropertiesChanged, WebKit::PlaybackSessionManagerProxy, void (WebKit::PlaybackSessionManagerProxy::*)(unsigned long long, bool, unsigned int, WTF::String)>(IPC::Decoder&, WebKit::PlaybackSessionManagerProxy*, void (WebKit::PlaybackSessionManagerProxy::*)(unsigned long long, bool, unsigned int, WTF::String)) + 73 (utility:753) 4 com.apple.WebKit 0x00000001056e24c5 IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) + 127 (MessageReceiverMap.cpp:123) 5 com.apple.WebKit 0x00000001058d9c0c WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 24 (WebProcessProxy.cpp:650) 6 com.apple.WebKit 0x00000001056d35c6 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 108 (memory:2714) 7 com.apple.WebKit 0x00000001056d6219 IPC::Connection::dispatchIncomingMessages() + 833 (memory:2734) 8 com.apple.JavaScriptCore 0x00000001043111f6 WTF::RunLoop::performWork() + 214 (Function.h:56) 9 com.apple.JavaScriptCore 0x0000000104311512 WTF::RunLoop::performWork(void*) + 34 (RunLoopCF.cpp:39) 10 com.apple.CoreFoundation 0x00007fffb22ff3e1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 11 com.apple.CoreFoundation 0x00007fffb22e065c __CFRunLoopDoSources0 + 556 12 com.apple.CoreFoundation 0x00007fffb22dfb46 __CFRunLoopRun + 934 13 com.apple.CoreFoundation 0x00007fffb22df544 CFRunLoopRunSpecific + 420 14 com.apple.Foundation 0x00007fffb3d10252 -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 277 15 WebKitTestRunner 0x000000010424c261 WTR::TestController::platformRunUntil(bool&, WTF::Seconds) + 191 (TestControllerCocoa.mm:204) 16 WebKitTestRunner 0x000000010424d321 WTR::TestInvocation::invoke() + 237 (TestInvocation.cpp:173) 17 WebKitTestRunner 0x0000000104242931 WTR::TestController::runTest(char const*) + 2205 (TestController.cpp:1379) 18 WebKitTestRunner 0x0000000104242c92 WTR::TestController::runTestingServerLoop() + 132 (TestController.cpp:1408) 19 WebKitTestRunner 0x000000010423d084 WTR::TestController::TestController(int, char const**) + 310 (TestController.cpp:147) 20 WebKitTestRunner 0x000000010422cd5c main + 657 (main.mm:68) 21 libdyld.dylib 0x00007fffc7eb1235 start + 1
Ryosuke Niwa
Comment 13 2018-10-01 18:59:54 PDT
Note You need to log in before you can comment on or make changes to this bug.