[webkit-reviews] review granted: [Bug 132537] [iOS WK2] Turn off position:fixed behavior when the keyboard is up : [Attachment 230774] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 132537: [iOS WK2] Turn off position:fixed behavior when the keyboard is up
https://bugs.webkit.org/show_bug.cgi?id=132537

Attachment 230774: Patch
https://bugs.webkit.org/attachment.cgi?id=230774&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=230774&action=review


Cool!

If you have time, it would be really nice to not freeze the layout in some
cases. If you can fix the caret issue, I would be happy to come with the
heuristic and fix the animation.

> Source/WebKit2/UIProcess/ios/WKContentView.mm:316
> +	   _lastFixedPositionRect =
fixedPositionRectFromExposedRect(unobscuredRect, [self bounds].size,
zoomScale);

We should rename fixedPositionRectFromExposedRect() to
fixedPositionRectFromUnobscuredRect() at some point.

[self bounds].size -> self.bounds.size.

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:2105
> +- (BOOL)_hasAssistedNode
> +{
> +    return _assistedNodeInformation.elementType != WKTypeNone;
> +}

Shouldn't you use "isEditable" and/or "isAssistingNode" instead?