[webkit-reviews] review granted: [Bug 237979] [iOS] Add `WKWebView` API to control CSS "small viewport" `sv*` and "large viewport" `lv*` units : [Attachment 455305] [fast-cq] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Devin Rousso <drousso at apple.com>'s
request for review:
Bug 237979: [iOS] Add `WKWebView` API to control CSS "small viewport" `sv*` and
"large viewport" `lv*` units
https://bugs.webkit.org/show_bug.cgi?id=237979

Attachment 455305: [fast-cq] Patch

https://bugs.webkit.org/attachment.cgi?id=455305&action=review




--- Comment #6 from Tim Horton <thorton at apple.com> ---
Comment on attachment 455305
  --> https://bugs.webkit.org/attachment.cgi?id=455305
[fast-cq] Patch

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

> Source/WebCore/page/FrameView.h:245
> +    WEBCORE_EXPORT void copyCSSViewportUnits(FrameView&);

Not sure about "copy" (would expect a method named "copy" to return a copy of
something, but it does not). Also "copy CSS Viewport units" isn't quite right
either, it's copying the viewport sizes for the units. IDK, I feel like the
name could use a re-think.

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.h:626
> + @discussion For apps with size-changing UI around the @link WKWebView
@/link, specify minimumViewportInset and

"size-changing UI" reads a little weird, but I assume you'll review this text
elsewhere

> Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:-3271
> -    _lastSentMinimumUnobscuredSize = newMinimumUnobscuredSize;
> -    _lastSentMaximumUnobscuredSize = newMaximumUnobscuredSize;

I have some vague and non-concrete fears about some of the deletions here re:
rotation in apps that use animated resize. We can chat elsewhere, but should
make sure we are careful.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:2444
> +    maximumUnobscuredSize.scale(1 /
m_viewportConfiguration.initialScaleIgnoringContentSize());

I don't remember the deal with initialScaleIgnoringContentSize vs.
initialScale; was this choice well-considered? Does it make sense in all the
weird viewport modes?