Bug 111532 - Clarify the size of a document view that causes FrameView to emit the resize event.
Summary: Clarify the size of a document view that causes FrameView to emit the resize ...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dongseong Hwang
URL:
Keywords:
Depends on: 80242 107424
Blocks: 81209 111408
  Show dependency treegraph
 
Reported: 2013-03-06 00:03 PST by Dongseong Hwang
Modified: 2013-07-09 15:52 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.01 KB, patch)
2013-03-06 00:07 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dongseong Hwang 2013-03-06 00:03:47 PST
The spec DOM Level 2 Events states that the resize event occurs when a document
view is resized.

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.

In addition, this removes delegatesScrolling check because delegatesScrolling is
not related to the size of a document view.
Comment 1 Dongseong Hwang 2013-03-06 00:07:03 PST
Created attachment 191665 [details]
Patch
Comment 2 Dongseong Hwang 2013-03-06 00:09:01 PST
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.
Comment 3 Kenneth Rohde Christiansen 2013-03-06 00:41:05 PST
Why isnt this tested?
Comment 4 Kenneth Rohde Christiansen 2013-03-06 00:43:17 PST
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! :-)
Comment 5 Allan Sandfeld Jensen 2013-03-06 02:10:54 PST
This is great, but can the subtle difference be tested. I guess it makes a difference on with fixedLayoutSize but disabled delegatedScrolling?
Comment 6 Alexandre Elias 2013-03-06 11:34:28 PST
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.
Comment 7 Dongseong Hwang 2013-03-14 22:36:16 PDT
(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?