Bug 192113

Summary: [iOSMac] Dropping text selections from web content into editable elements crashes the web process
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: HTML EditingAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, megan_gardner, rniwa, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
rniwa: review+
Patch for EWS
none
Patch for EWS none

Description Wenson Hsieh 2018-11-28 15:19:05 PST
SSIA
Comment 1 Radar WebKit Bug Importer 2018-11-28 15:19:45 PST
<rdar://problem/46323701>
Comment 2 Wenson Hsieh 2018-11-28 15:21:37 PST
(In reply to Wenson Hsieh from comment #0)
> SSIA

On second thought, there's a bit more to say here :| The following interactions either crash the web process or fail silently:

• Drag text selection from iOSMac and drop into a web view in macOS.
• Drag text selection from a web view in macOS and drop into an editable area in iOSMac.
• Drag and drop within editable areas in iOSMac.
Comment 3 Wenson Hsieh 2018-11-28 15:39:37 PST
Created attachment 355935 [details]
Patch
Comment 4 Ryosuke Niwa 2018-11-28 15:58:00 PST
Comment on attachment 355935 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        In iOSMac, registering invalid UTIs on NSItemProvider when starting a drag or handling a drop fails gracefully.

You mean this doesn't fail gracefully???

> Source/WebCore/platform/ios/PasteboardIOS.mm:200
> -    if ([type isEqualToString:(NSString *)kUTTypeFlatRTFD]) {
> +    if ([type isEqualToString:(__bridge NSString *)kUTTypeFlatRTFD]) {

Can we compile out this code in iOSMac?
Comment 5 Ryosuke Niwa 2018-11-28 15:58:35 PST
We should make sure we never try to read RTF / FlatRTF even when there is no WebArchive though.
Comment 6 Wenson Hsieh 2018-11-28 16:07:48 PST
Comment on attachment 355935 [details]
Patch

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

>> Source/WebCore/ChangeLog:9
>> +        In iOSMac, registering invalid UTIs on NSItemProvider when starting a drag or handling a drop fails gracefully.
> 
> You mean this doesn't fail gracefully???

Ah, I just meant it fails but doesn't cause a crash or throw an exception. I'll reword to make this clearer.

>> Source/WebCore/platform/ios/PasteboardIOS.mm:200
>> +    if ([type isEqualToString:(__bridge NSString *)kUTTypeFlatRTFD]) {
> 
> Can we compile out this code in iOSMac?

Yep! As we discussed, I'll also add a RELEASE_ASSERT in the codepath where we attempt to use UIFoundation to try and convert an attributed string to markup.
Comment 7 Wenson Hsieh 2018-11-28 18:38:22 PST
(In reply to Wenson Hsieh from comment #6)
> Comment on attachment 355935 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=355935&action=review
> 
> >> Source/WebCore/ChangeLog:9
> >> +        In iOSMac, registering invalid UTIs on NSItemProvider when starting a drag or handling a drop fails gracefully.
> > 
> > You mean this doesn't fail gracefully???
> 
> Ah, I just meant it fails but doesn't cause a crash or throw an exception.
> I'll reword to make this clearer.
> 
> >> Source/WebCore/platform/ios/PasteboardIOS.mm:200
> >> +    if ([type isEqualToString:(__bridge NSString *)kUTTypeFlatRTFD]) {
> > 
> > Can we compile out this code in iOSMac?
> 
> Yep! As we discussed, I'll also add a RELEASE_ASSERT in the codepath where
> we attempt to use UIFoundation to try and convert an attributed string to
> markup.

(Actually, I think it makes more sense to just guard out the call site on iOSMac, so that it just fails gracefully instead of crashing.)
Comment 8 Wenson Hsieh 2018-11-28 19:54:57 PST
Created attachment 355967 [details]
Patch for EWS
Comment 9 Wenson Hsieh 2018-11-28 20:06:23 PST
Created attachment 355970 [details]
Patch for EWS
Comment 10 WebKit Commit Bot 2018-11-28 21:25:14 PST
Comment on attachment 355970 [details]
Patch for EWS

Clearing flags on attachment: 355970

Committed r238661: <https://trac.webkit.org/changeset/238661>