Bug 220537 - Clean up some mainframe scroll snap tests
Summary: Clean up some mainframe scroll snap tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Robinson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-01-12 01:01 PST by Martin Robinson
Modified: 2021-01-12 08:31 PST (History)
2 users (show)

See Also:

Attachments
Patch (15.27 KB, patch)
2021-01-12 01:06 PST, Martin Robinson 		no flags Details | Formatted Diff | Diff
Patch (15.61 KB, patch)
2021-01-12 07:51 PST, Martin Robinson 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2021-01-12 01:01:29 PST 
Some mainframe scroll snap tests include dead code and are written in a way that can hide some failures. This bug tracks cleaning up those tests.
Comment 1 Martin Robinson 2021-01-12 01:06:19 PST 
Created attachment 417439 [details]
Patch
Comment 2 Frédéric Wang (:fredw) 2021-01-12 06:17:27 PST 
Comment on attachment 417439 [details]
Patch

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

> LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-horizontal-legacy.html:40
> +                expectTrue(document.scrollingElement.scrollLeft == window.innerWidth, "div honored snap points.");

This (and below) is no longer comparing against scrollPositionBeforeSnap == document.scrollingElement.scrollLeft ; is it intentional?
Comment 3 Martin Robinson 2021-01-12 06:19:56 PST 
(In reply to Frédéric Wang (:fredw) from comment #2)
> Comment on attachment 417439 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=417439&action=review
> 
> > LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-horizontal-legacy.html:40
> > +                expectTrue(document.scrollingElement.scrollLeft == window.innerWidth, "div honored snap points.");
> 
> This (and below) is no longer comparing against scrollPositionBeforeSnap ==
> document.scrollingElement.scrollLeft ; is it intentional?

Yes, instead of sampling the scroll position before the tests, the code now relies on all scroll positions being fixed values. This should always be the case. In this way the tests are a little stronger, I think.
Comment 4 Frédéric Wang (:fredw) 2021-01-12 06:23:14 PST 
Comment on attachment 417439 [details]
Patch

OK, maybe worth explaining it in the changelog?
Comment 5 Martin Robinson 2021-01-12 07:51:50 PST 
Created attachment 417456 [details]
Patch
Comment 6 Martin Robinson 2021-01-12 07:52:22 PST 
(In reply to Frédéric Wang (:fredw) from comment #4)
> Comment on attachment 417439 [details]
> Patch
> 
> OK, maybe worth explaining it in the changelog?

Sure. I've updated the ChangeLog now.
Comment 7 EWS 2021-01-12 08:30:45 PST 
Committed r271403: <https://trac.webkit.org/changeset/271403>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417456 [details].
Comment 8 Radar WebKit Bug Importer 2021-01-12 08:31:29 PST 
<rdar://problem/73041906>