[webkit-reviews] review granted: [Bug 133162] [iOS][WK2] Add support for minimal-ui viewports : [Attachment 231933] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 133162: [iOS][WK2] Add support for minimal-ui viewports
https://bugs.webkit.org/show_bug.cgi?id=133162

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=231933&action=review


> Source/WebCore/ChangeLog:14
> +	   Initially, the page is initialized with resetMinimalUI() and start
with a normal layout.

starts with

> Source/WebCore/ChangeLog:18
> +	   layout size. Any layout after that take into account the minimal-ui.


takes minimal-ui into account.

> Source/WebCore/page/ViewportConfiguration.cpp:120
> +    if (m_pageDidFinishDocumentLoad)
> +	   return;

No point having these lines.

> Source/WebCore/page/ViewportConfiguration.cpp:286
>  int ViewportConfiguration::layoutWidth() const

This should return LayoutUnits at some point?

> Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:99
> +// Define the smallest size a page take with a regular.

With a regular what?

> Source/WebKit2/WebProcess/WebPage/WebPage.h:253
> +    void didFinishDocumentLoad(WebFrame*);
>      void didFinishLoad(WebFrame*);

What's the difference? Is the first like DOMContentLoaded?

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:1992
>     
resetTextAutosizingBeforeLayoutIfNeeded(m_viewportConfiguration.minimumLayoutSi
ze(), minimumLayoutSize);
> +   
resetTextAutosizingBeforeLayoutIfNeeded(m_viewportConfiguration.minimumLayoutSi
zeForMinimalUI(), minimumLayoutSizeForMinimalUI);

Do we have to call resetTextAutosizingBeforeLayoutIfNeeded twice?