Bug 111532

Created attachment 191665 [details] Patch

This patch is the follow-up patch of Bug 80242. r141053 that the changelog refers to is Bug 107424. I tested only Qt and EFL ports, so I need various feedback.

Why isnt this tested?

Comment on attachment 191665 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191665&action=review > Source/WebCore/ChangeLog:13 > + After r141053, FrameView::unscaledVisibleContentSize stands for a document view > + size. When turning on fixed layout, fixed layout size stands for a document view > + size. This patch applies the term of the spec to FrameView exactly. view? Not viewport? I don't get this. With fixed layout, the fixed layout size is the size of the document, not the viewport > Source/WebCore/ChangeLog:19 > + > + Covered by existing tests: fast/events/resize-events.html > + Sorry, it must be early yet. I missed this one! :-)

This is great, but can the subtle difference be tested. I guess it makes a difference on with fixedLayoutSize but disabled delegatedScrolling?

I think there's a bit of an open question about whether, in the presence of page scale, we should report to Javascript (1) the true user-visible scaled viewport, or (2) the fixed layout size (and a scroll offset properly clamped to go with it). The specs are ambiguous about this since page scale didn't exist back then. I'm inclined to favor (2) since that's more backwards compatible, and it looks like this patch is moving in that direction, so it's okay with me. Though in that case, to be more consistent, it seems like the viewport size sent to Javascript should also changed be unscaledVisibleContentSize.

(In reply to comment #5) > This is great, but can the subtle difference be tested. I guess it makes a difference on with fixedLayoutSize but disabled delegatedScrolling? Thank you for review! Yes, you're right: the port with fixedLayoutSize but disabled delegatedScrolling now keeps fixedLayoutSize() as a m_lastViewportSize. the port can be tested using fast/events/resize-events.html also. I think this change goes to right direction. I think a document view is layout viewport that http://www.quirksmode.org/mobile/viewports2.html said, and when enabling fixed layout, layout viewport would be fixed layout size. (In reply to comment #6) > I think there's a bit of an open question about whether, in the presence of page scale, we should report to Javascript (1) the true user-visible scaled viewport, or (2) the fixed layout size (and a scroll offset properly clamped to go with it). The specs are ambiguous about this since page scale didn't exist back then. Thank you for good open questions! In my understanding based on http://www.quirksmode.org/mobile/viewports2.html 1. If a user uses window.innerWidth/Height, WebKit should return user-visible scaled viewport, which is visibleContentRect(IncludeScrollbars).size(). 2. If a user uses document.documentElement.clientWidth/Height, WebKit should return unscaledVisibleContentSize. And if enabling fixed layout, WebKit should return fixed layout size. > I'm inclined to favor (2) since that's more backwards compatible, and it looks like this patch is moving in that direction, so it's okay with me. Though in that case, to be more consistent, it seems like the viewport size sent to Javascript should also changed be unscaledVisibleContentSize. I manually test http://www.quirksmode.org/m/tests/widthtest.html on Qt WK2, both window.innerWidth and document.documentElement.clientWidth work well. If do you think we should change anything, could you explain more?