Bug 154300 – Add tests for iframe and overflow scrollability after navigating back

• Component: New Bugs
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

Add tests for iframe and overflow scrollability after navigating back

[Simon Fraser (smfr)]

Created attachment 271452 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 271458 [details]
Patch

[Brent Fulgham]

Comment on attachment 271458 [details]
Patch

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

r=me, but please consider only checking for "changed" rather than specific scroll position numbers.

> LayoutTests/fast/scrolling/iframe-scrollable-after-back.html:35
> +            logResult('Scrolled; scrollTop is ' + document.getElementById('iframe').contentDocument.scrollingElement.scrollTop);

This might be flaky on different machines (e.g., with different scroll precision). You might just confirm that the scrollTop changed from whatever value you see at the start of the test.

[Simon Fraser (smfr)]

(In reply to comment #3)
> Comment on attachment 271458 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=271458&action=review
> 
> r=me, but please consider only checking for "changed" rather than specific
> scroll position numbers.
> 
> > LayoutTests/fast/scrolling/iframe-scrollable-after-back.html:35
> > +            logResult('Scrolled; scrollTop is ' + document.getElementById('iframe').contentDocument.scrollingElement.scrollTop);
> 
> This might be flaky on different machines (e.g., with different scroll
> precision). You might just confirm that the scrollTop changed from whatever
> value you see at the start of the test.

If it is, that's a problem with the test harness.

[Build Bot]

Comment on attachment 271458 [details]
Patch

Attachment 271458 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/841496

New failing tests:
fast/scrolling/overflow-scrollable-after-back.html
fast/scrolling/iframe-scrollable-after-back.html

[Build Bot]

Created attachment 271469 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 271458 [details]
Patch

Attachment 271458 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/841508

New failing tests:
fast/scrolling/overflow-scrollable-after-back.html
fast/scrolling/iframe-scrollable-after-back.html

[Build Bot]

Created attachment 271471 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Simon Fraser (smfr)]

https://trac.webkit.org/r196665

[Brent Fulgham]

(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 271458 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=271458&action=review
> > 
> > r=me, but please consider only checking for "changed" rather than specific
> > scroll position numbers.
> > 
> > > LayoutTests/fast/scrolling/iframe-scrollable-after-back.html:35
> > > +            logResult('Scrolled; scrollTop is ' + document.getElementById('iframe').contentDocument.scrollingElement.scrollTop);
> > 
> > This might be flaky on different machines (e.g., with different scroll
> > precision). You might just confirm that the scrollTop changed from whatever
> > value you see at the start of the test.
> 
> If it is, that's a problem with the test harness.

Ermmmm (from test failures):

 Sending wheel event; scrollTop is 0
-Scrolled; scrollTop is 20
+Scrolled; scrollTop is 28

[Simon Fraser (smfr)]

I skipped on WK1 when landing.

[Carlos Garcia Campos]

(In reply to comment #10)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Comment on attachment 271458 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=271458&action=review
> > > 
> > > r=me, but please consider only checking for "changed" rather than specific
> > > scroll position numbers.
> > > 
> > > > LayoutTests/fast/scrolling/iframe-scrollable-after-back.html:35
> > > > +            logResult('Scrolled; scrollTop is ' + document.getElementById('iframe').contentDocument.scrollingElement.scrollTop);
> > > 
> > > This might be flaky on different machines (e.g., with different scroll
> > > precision). You might just confirm that the scrollTop changed from whatever
> > > value you see at the start of the test.
> > 
> > If it is, that's a problem with the test harness.
> 
> Ermmmm (from test failures):
> 
>  Sending wheel event; scrollTop is 0
> -Scrolled; scrollTop is 20
> +Scrolled; scrollTop is 28

Similar failures in GTK+ where minimum scroll is 40.

[Simon Fraser (smfr)]

I'll fix the tests.

[Carlos Garcia Campos]

(In reply to comment #13)
> I'll fix the tests.

Thanks!, I also think that just checking that the view did actually scroll should be enough as Brent suggested, how much it scrolled doesn't really matter in this test.

[Simon Fraser (smfr)]

Tests fixed in https://trac.webkit.org/r196719