[webkit-reviews] review granted: [Bug 193494] [iOS] Content offset jumps erratically when autoscrolling near scroll view content inset areas : [Attachment 359379] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jan 17 13:45:08 PST 2019
Tim Horton <thorton at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 193494: [iOS] Content offset jumps erratically when autoscrolling near
scroll view content inset areas
https://bugs.webkit.org/show_bug.cgi?id=193494
Attachment 359379: Patch
https://bugs.webkit.org/attachment.cgi?id=359379&action=review
--- Comment #2 from Tim Horton <thorton at apple.com> ---
Comment on attachment 359379
--> https://bugs.webkit.org/attachment.cgi?id=359379
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=359379&action=review
> Source/WebKit/ChangeLog:26
> + We don't care about binary compatibility with iOS 10 and below
anymore, so we should change these >= iOS 11
> + target checks to simply `PLATFORM(IOS)`.
This is about source compatibility, not bincompat, right?
> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2979
> + auto scrollViewOriginIncludingInset = UIEdgeInsetsInsetRect([_scrollView
bounds], computedContentInsetUnadjustedForKeyboard).origin;
Should we stash aside [_scrollView bounds] and contentSize to avoid all these
messages?
> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2983
> + if (scrollViewInsets.left > 0 && scrollViewOriginIncludingInset.x < 0)
This all could probably use a comment. Also I am looking at UIGeometry headers
longingly wishing there were a better way to write all this.
> Tools/WebKitTestRunner/TestController.cpp:1270
> + else if (key == "topContentInset")
> + testOptions.topContentInset = std::stod(value);
Please don't propagate the silly "topContentInset" thing. This will be
confusing when getting rid of the Mac notion.
I wonder if we can actually put dots here? like contentInset.top in the key
name?
More information about the webkit-reviews
mailing list