[webkit-reviews] review granted: [Bug 58102] REGRESSION (WebKit2): AppKit thinks that web views don't support DocumentAccess : [Attachment 88742] proposed fix

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 58102: REGRESSION (WebKit2): AppKit thinks that web views don't support
DocumentAccess
https://bugs.webkit.org/show_bug.cgi?id=58102

Attachment 88742: proposed fix
https://bugs.webkit.org/attachment.cgi?id=88742&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=88742&action=review

> Source/WebKit2/UIProcess/API/mac/WKView.mm:1050
> +	   LOG(TextInput, "insertText:\"%@\" replacementRange:(%u, %u)",
isAttributedString ? [string string] : string, replacementRange.location,
replacementRange.length);

%u is not the appropriate formatting specifier for NSUInteger; it’s right for
unsigned and wrong for NSUInteger. The simplest workaround is to cast to
unsigned.

>> Source/WebKit2/UIProcess/API/mac/WKView.mm:1061

> 
> Is this comment still applicable?

I believe it is.

> Source/WebKit2/UIProcess/API/mac/WKView.mm:1348
> -- (NSAttributedString *)attributedSubstringFromRange:(NSRange)nsRange
> +- (NSAttributedString *)attributedSubstringForProposedRange:(NSRange)nsRange
actualRange:(NSRangePointer)actualRange

The name nsRange is not good for the first argument. Leaving it alone makes the
patch smaller, but other than that I see no reason to leave it with this name.

> Source/WebKit2/UIProcess/API/mac/WKView.mm:1383
> +- (NSRect)firstRectForCharacterRange:(NSRange)theRange
actualRange:(NSRangePointer)actualRange

The name theRange is not good for the first argument. Leaving it alone makes
the patch smaller, but other than that I see no reason to leave it with this
name.