WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
220537
Clean up some mainframe scroll snap tests
https://bugs.webkit.org/show_bug.cgi?id=220537
Summary
Clean up some mainframe scroll snap tests
Martin Robinson
Reported
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.
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 attachment
proposed patch, testcase, etc.
Martin Robinson
Comment 1
2021-01-12 01:06:19 PST
Created
attachment 417439
[details]
Patch
Frédéric Wang (:fredw)
Comment 2
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?
Martin Robinson
Comment 3
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.
Frédéric Wang (:fredw)
Comment 4
2021-01-12 06:23:14 PST
Comment on
attachment 417439
[details]
Patch OK, maybe worth explaining it in the changelog?
Martin Robinson
Comment 5
2021-01-12 07:51:50 PST
Created
attachment 417456
[details]
Patch
Martin Robinson
Comment 6
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.
EWS
Comment 7
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]
.
Radar WebKit Bug Importer
Comment 8
2021-01-12 08:31:29 PST
<
rdar://problem/73041906
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug