Bug 142582 – Scroll snap points are not supported on iframe content

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Brent Fulgham]

See attached test case for an example. The <iframe> element should support the scroll snap point feature.

[Brent Fulgham]

Created attachment 248423 [details]
Manual test case for iframe scroll snap point

[Radar WebKit Bug Importer]

<rdar://problem/20121319>

[Brent Fulgham]

Created attachment 248691 [details]
Updated example

[Brent Fulgham]

The scroll snap points were not being applied to the iframe contents because the code that sets up the scroll snap point content is not called for iframes.

To correct this, we need to make sure the snap offsets are set during post-frame layout for iframes. We also need to make sure (on Mac) that the scroll animator and timers are updated.

[Brent Fulgham]

Created attachment 248696 [details]
Patch

[Build Bot]

Comment on attachment 248696 [details]
Patch

Attachment 248696 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4684014753415168

New failing tests:
http/tests/security/storage-blocking-loosened-plugin.html
http/tests/security/cross-origin-plugin.html
platform/mac-wk2/tiled-drawing/scrolling/frames/coordinated-frame-in-fixed.html
http/tests/navigation/anchor-frames-gbk.html
http/tests/security/cross-origin-plugin-allowed.html
plugins/npruntime/object-from-destroyed-plugin-in-subframe.html
http/tests/security/mixedContent/insecure-plugin-in-iframe.html
http/tests/security/contentSecurityPolicy/object-src-none-blocked.html
plugins/plugin-remove-subframe.html
plugins/plugin-document-back-forward.html
fast/frames/sandboxed-iframe-navigation-allowed.html
http/tests/plugins/create-v8-script-objects.html
plugins/resize-from-plugin.html
http/tests/security/contentSecurityPolicy/object-src-none-allowed.html
platform/mac-wk2/tiled-drawing/scrolling/frames/fixed-inside-frame.html
fast/dynamic/paused-event-dispatch.html
platform/mac-wk2/tiled-drawing/scrolling/frames/scroll-region-after-frame-layout.html
http/tests/plugins/cross-frame-object-access.html
http/tests/security/xssAuditor/object-src-inject.html
fast/frames/sandboxed-iframe-about-blank.html
http/tests/plugins/third-party-cookie-accept-policy.html
fast/replaced/frame-removed-during-resize-smaller.html
platform/mac-wk2/tiled-drawing/scrolling/frames/coordinated-frame.html
http/tests/security/storage-blocking-strengthened-plugin.html
fast/events/resize-subframe.html

[Build Bot]

Created attachment 248698 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Brent Fulgham]

Created attachment 248701 [details]
Patch

[Brent Fulgham]

I'm surprised by these test failures, given that the new code just sets scroll snap state in a few member variables.

[Build Bot]

Comment on attachment 248701 [details]
Patch

Attachment 248701 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4747936214810624

New failing tests:
http/tests/security/storage-blocking-loosened-plugin.html
http/tests/security/cross-origin-plugin.html
platform/mac-wk2/tiled-drawing/scrolling/frames/coordinated-frame-in-fixed.html
http/tests/navigation/anchor-frames-gbk.html
http/tests/security/cross-origin-plugin-allowed.html
plugins/npruntime/object-from-destroyed-plugin-in-subframe.html
http/tests/security/mixedContent/insecure-plugin-in-iframe.html
http/tests/security/contentSecurityPolicy/object-src-none-blocked.html
plugins/plugin-remove-subframe.html
plugins/plugin-document-back-forward.html
fast/frames/sandboxed-iframe-navigation-allowed.html
http/tests/plugins/create-v8-script-objects.html
plugins/resize-from-plugin.html
http/tests/security/contentSecurityPolicy/object-src-none-allowed.html
platform/mac-wk2/tiled-drawing/scrolling/frames/fixed-inside-frame.html
fast/dynamic/paused-event-dispatch.html
platform/mac-wk2/tiled-drawing/scrolling/frames/scroll-region-after-frame-layout.html
http/tests/plugins/cross-frame-object-access.html
http/tests/security/xssAuditor/object-src-inject.html
fast/frames/sandboxed-iframe-about-blank.html
http/tests/plugins/third-party-cookie-accept-policy.html
fast/replaced/frame-removed-during-resize-smaller.html
platform/mac-wk2/tiled-drawing/scrolling/frames/coordinated-frame.html
http/tests/security/storage-blocking-strengthened-plugin.html
fast/events/resize-subframe.html

[Build Bot]

Created attachment 248705 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Brent Fulgham]

Created attachment 248707 [details]
Patch (v3 correct some test failures)

[Simon Fraser (smfr)]

Comment on attachment 248707 [details]
Patch (v3 correct some test failures)

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

> Source/WebCore/page/FrameView.cpp:2986
> +#if PLATFORM(MAC)
> +        if (ScrollAnimator* scrollAnimator = existingScrollAnimator())
> +            return scrollAnimator->updateScrollAnimatorsAndTimers();
> +#endif

Why is this Mac-specific?

[Brent Fulgham]

Comment on attachment 248707 [details]
Patch (v3 correct some test failures)

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

>> Source/WebCore/page/FrameView.cpp:2986
>> +#endif
> 
> Why is this Mac-specific?

You are right; we should make sure it's up-to-date for iOS as well.

[Simon Fraser (smfr)]

Comment on attachment 248707 [details]
Patch (v3 correct some test failures)

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

> LayoutTests/css3/scroll-snap/resources/iframe-content.html:236
> +        top.succeeded = top.succeeded && shouldMatch("window.getComputedStyle(noInitial).getPropertyValue('-webkit-scroll-snap-points-x')", "repeat(100%)");
> +        top.succeeded = top.succeeded && shouldMatch("window.getComputedStyle(noInitial).getPropertyValue('-webkit-scroll-snap-points-y')", "repeat(100%)");
> +        top.succeeded = top.succeeded && shouldMatch("noInitial.style['-webkit-scroll-snap-destination']", "100% 100%");
> +        top.succeeded = top.succeeded && shouldMatch("window.getComputedStyle(noInitial).getPropertyValue('-webkit-scroll-snap-destination')", "100% 100%");
> +        top.succeeded = top.succeeded && shouldMatch("noInitial.style['-webkit-scroll-snap-coordinate']", "0% 0%");
> +        top.succeeded = top.succeeded && shouldMatch("window.getComputedStyle(noInitial).getPropertyValue('-webkit-scroll-snap-coordinate')", "0% 0%");
> +
> +        var initialType = document.getElementById('initialType');
> +        top.succeeded = top.succeeded && shouldMatch("initialType.style['-webkit-scroll-snap-type']", "initial");
> +        top.succeeded = top.succeeded && shouldMatch("window.getComputedStyle(initialType).getPropertyValue('-webkit-scroll-snap-type')", "none");
> +        top.succeeded = top.succeeded && shouldMatch("window.getComputedStyle(initialType).getPropertyValue('-webkit-scroll-snap-points-x')", "repeat(100%)");
> +        top.succeeded = top.succeeded && shouldMatch("window.getComputedStyle(initialType).getPropertyValue('-webkit-scroll-snap-points-y')", "repeat(100%)");
> +        top.succeeded = top.succeeded && shouldMatch("initialType.style['-webkit-scroll-snap-destination']", "0% 0%");
> +        top.succeeded = top.succeeded && shouldMatch("window.getComputedStyle(initialType).getPropertyValue('-webkit-scroll-snap-destination')", "0% 0%");
> +        top.succeeded = top.succeeded && shouldMatch("initialType.style['-webkit-scroll-snap-coordinate']", "100% 100%");
> +        top.succeeded = top.succeeded && shouldMatch("window.getComputedStyle(initialType).getPropertyValue('-webkit-scroll-snap-coordinate')", "100% 100%");
> +
> +        var initialXPoints = document.getElementById('initialXPoints');
> +        top.succeeded = top.succeeded && shouldMatch("window.getComputedStyle(initialXPoints).getPropertyValue('-webkit-scroll-snap-type')", "mandatory");
> +        top.succeeded = top.succeeded && shouldMatch("initialXPoints.style['-webkit-scroll-snap-points-x']", "initial");

Does this subframe really need all of this? Please reduce it to the minimum required to test the bug (probably one line).

[Brent Fulgham]

Committed r181522: <http://trac.webkit.org/changeset/181522>

(In reply to comment #14)
> Comment on attachment 248707 [details]
> Patch (v3 correct some test failures)
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248707&action=review
> 
> >> Source/WebCore/page/FrameView.cpp:2986
> >> +#endif
> > 
> > Why is this Mac-specific?
> 
> You are right; we should make sure it's up-to-date for iOS as well.

updateScrollAnimatorsAndTimers() is Mac-only. Removing the conditional in the version of the patch committed in r181522 caused the iOS build to break. I restored it in <http://trac.webkit.org/r181524>.