Bug 167541

Summary: [WK1] Do not prevent the drag client from initializing on Mac
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: WebKit Misc.Assignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: mitz, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch mitz: review+

Description Wenson Hsieh 2017-01-27 20:26:48 PST
Do not prevent the drag client from initializing on Mac. Fix fallout from refactoring drag and drop on the Mac.
Comment 1 Wenson Hsieh 2017-01-27 20:40:10 PST
Created attachment 299994 [details]
Patch
Comment 2 mitz 2017-01-27 20:47:57 PST
Comment on attachment 299994 [details]
Patch

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

> Source/WebKit/mac/ChangeLog:4
> +        [WK1] Do not prevent the drag client from initializing on Mac
> +        https://bugs.webkit.org/show_bug.cgi?id=167541

It would be nice if the bug title described a symptom (an assertion failure when closing a WebView initialized with -initSimpleHTMLDocumentWithStyle:frame:preferences:groupName:) rather than the change.

> Source/WebKit/mac/WebCoreSupport/WebDragClient.mm:41
> +#if PLATFORM(MAC)
>  #import "WebNSPasteboardExtras.h"
> +#endif

#if blocks should not be interspersed with the sorted import directives, but instead go separately below.

> Source/WebKit/mac/WebCoreSupport/WebDragClient.mm:62
> +    UNUSED_PARAM(m_webView);

Why does a member variable need to be treated as an unused parameter?

> Source/WebKit/mac/WebCoreSupport/WebDragClient.mm:157
> +
> +

One empty line is enough.

> Source/WebKit/mac/WebCoreSupport/WebDragClient.mm:181
> +#endif // PLATFORM(MAC)

I thought we had decided to avoid comments on #endif after #else, due to the ambiguity.
Comment 3 Wenson Hsieh 2017-01-28 11:45:02 PST
Comment on attachment 299994 [details]
Patch

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

Thanks!

>> Source/WebKit/mac/ChangeLog:4
>> +        https://bugs.webkit.org/show_bug.cgi?id=167541
> 
> It would be nice if the bug title described a symptom (an assertion failure when closing a WebView initialized with -initSimpleHTMLDocumentWithStyle:frame:preferences:groupName:) rather than the change.

Added.

>> Source/WebKit/mac/WebCoreSupport/WebDragClient.mm:41
>> +#endif
> 
> #if blocks should not be interspersed with the sorted import directives, but instead go separately below.

Ok! Fixed.

>> Source/WebKit/mac/WebCoreSupport/WebDragClient.mm:62
>> +    UNUSED_PARAM(m_webView);
> 
> Why does a member variable need to be treated as an unused parameter?

This will throw a compiler error when building, since m_webView is private.

>> Source/WebKit/mac/WebCoreSupport/WebDragClient.mm:157
>> +
> 
> One empty line is enough.

Fixed.

>> Source/WebKit/mac/WebCoreSupport/WebDragClient.mm:181
>> +#endif // PLATFORM(MAC)
> 
> I thought we had decided to avoid comments on #endif after #else, due to the ambiguity.

Ah, that makes sense. Fixed.
Comment 4 Wenson Hsieh 2017-01-28 11:45:24 PST
Committed <https://trac.webkit.org/changeset/211323>
Comment 5 Wenson Hsieh 2017-01-28 11:46:15 PST
<rdar://problem/30247109>