Bug 167541 - [WK1] Do not prevent the drag client from initializing on Mac
Summary: [WK1] Do not prevent the drag client from initializing on Mac
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-01-27 20:26 PST by Wenson Hsieh
Modified: 2017-01-28 11:46 PST (History)
2 users (show)

See Also:


Attachments
Patch (4.22 KB, patch)
2017-01-27 20:40 PST, Wenson Hsieh
mitz: review+
Details | Formatted Diff | Diff

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