[webkit-reviews] review denied: [Bug 174362] window.innerWidth/height shouldn't change under pinch zoom : [Attachment 328488] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 5 14:09:16 PST 2017


Simon Fraser (smfr) <simon.fraser at apple.com> has denied Ali Juma
<ajuma at chromium.org>'s request for review:
Bug 174362: window.innerWidth/height shouldn't change under pinch zoom
https://bugs.webkit.org/show_bug.cgi?id=174362

Attachment 328488: Patch

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




--- Comment #33 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 328488
  --> https://bugs.webkit.org/attachment.cgi?id=328488
Patch

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

>>> Source/WebCore/page/DOMWindow.cpp:1270
>>> +	     return static_cast<int>((view->layoutViewportRect().height() +
scrollbarHeight) * view->layoutViewportToClientScaleFactor());
>> 
>> I'm still a bit surprised that we need code here to deal with platform vs.
non-platform scrollbars. What does scrollbar->occupiedWidth() return if there
are platform widget scrollbars?
> 
> When there's a platform widget, ScrollView's own scrollbars are null so
there's no scrollbar to call occupiedWidth() on. (ScrollView's scrollbars get
created in ScrollView::updateScrollbars, but when there's a platform widget
that method returns immediately without doing any work.)

OK. Can you push this code down into ScrollVIew? DOMWindow should not be in the
position of having to think about platform scrollbars.

Maybe an enum param to horizontalScrollbarIntrusion(), like
IncludePlatformWidgetScrollbars or something.


More information about the webkit-reviews mailing list