Bug 192113 - [iOSMac] Dropping text selections from web content into editable elements crashes the web process
Summary: [iOSMac] Dropping text selections from web content into editable elements cra...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-11-28 15:19 PST by Wenson Hsieh
Modified: 2018-11-28 21:25 PST (History)
7 users (show)

See Also:


Attachments
Patch (13.65 KB, patch)
2018-11-28 15:39 PST, Wenson Hsieh
rniwa: review+
Details | Formatted Diff | Diff
Patch for EWS (15.34 KB, patch)
2018-11-28 19:54 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch for EWS (15.33 KB, patch)
2018-11-28 20:06 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>