Bug 58102 – REGRESSION (WebKit2): AppKit thinks that web views don't support DocumentAccess

• Component: HTML Editing
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

This works in WebKit1 because it implements textStorage method, but a better way to convince AppKit would be to switch to NSTextInputClient protocol. In the future, this could let us do performance optimizations too.

Patch forthcoming.

<rdar://problem/9223246>

[Alexey Proskuryakov]

Created attachment 88742 [details]
proposed fix

We could do this in WebHTMLView, but it has a huge baggage of hackish workarounds that changing the behavior by switching to a new protocol is super scary. The proposed alternative of having the view classes diverge is slightly less scary.

[Enrica Casucci]

Comment on attachment 88742 [details]
proposed fix

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

The overall implementation looks good to me.

> Source/WebKit2/UIProcess/API/mac/WKView.mm:1061
>          // FIXME: We ignore most attributes from the string, so for example inserting from Character Palette loses font and glyph variation data.

Is this comment still applicable?

> Source/WebKit2/UIProcess/API/mac/WKView.mm:1324
>          // FIXME: We ignore most attributes from the string, so an input method cannot specify e.g. a font or a glyph variation.

Same as above. Is this comment still applicable?

> Source/WebKit2/UIProcess/API/mac/WKView.mm:1399
> +

unnecessary change

[Alexey Proskuryakov]

> Is this comment still applicable?

Yes, we ignore most attributes.

[Darin Adler]

Comment on attachment 88742 [details]
proposed fix

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.

[Alexey Proskuryakov]

> %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.

This is what we do elsewhere in WebHTMLView and WKView, I would prefer to not introduce an inconsistency, and to not make global changes in this patch.

> 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.
<...>
> 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.

I'm not sure what name would be better. AppKit uses "aRange" for these arguments, which is no more speaking than our names, and doesn't fit our common style.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/changeset/83326>.