Bug 197211

Summary: [iOS] Element::focus and Element::scrollIntoView do not clamp scroll positions
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: ScrollingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, ews, koivisto, rniwa, ryanhaddad, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=170982
Attachments:
Description Flags
Fixes the bug
none
Archive of layout-test-results from ews103 for mac-highsierra
none
Archive of layout-test-results from ews107 for mac-highsierra-wk2
none
Archive of layout-test-results from ews114 for mac-highsierra
none
Archive of layout-test-results from ews122 for ios-simulator-wk2
none
Archive of layout-test-results from ews200 for win-future
none
Fixes the bug
none
Fixes the bug
none
Archive of layout-test-results from ews122 for ios-simulator-wk2
none
Fixed the test simon.fraser: review+

Description Ryosuke Niwa 2019-04-23 14:10:11 PDT
Element::focus and Element::scrollIntoView don't clamp scroll positions.
This results in scrollTop to report bogus numbers like -1000.

<rdar://problem/49784711>
Comment 1 Ryosuke Niwa 2019-04-23 14:21:47 PDT
Created attachment 368063 [details]
Fixes the bug
Comment 2 Build Bot 2019-04-23 15:06:39 PDT
Comment on attachment 368063 [details]
Fixes the bug

Attachment 368063 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11975688

New failing tests:
fast/scrolling/programmatic-document-rtl-scrollIntoView.html
fast/dynamic/anchor-lock.html
fast/transforms/selection-bounds-in-transformed-view.html
imported/w3c/web-platform-tests/shadow-dom/scroll-to-the-fragment-in-shadow-tree.html
fast/visual-viewport/zoomed-scroll-into-view-fixed.html
fast/scrolling/scroll-to-anchor-zoomed-header.html
Comment 3 Build Bot 2019-04-23 15:06:40 PDT
Created attachment 368068 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 4 Build Bot 2019-04-23 15:33:00 PDT
Comment on attachment 368063 [details]
Fixes the bug

Attachment 368063 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11975760

New failing tests:
fast/scrolling/programmatic-document-rtl-scrollIntoView.html
fast/dynamic/anchor-lock.html
fast/transforms/selection-bounds-in-transformed-view.html
imported/w3c/web-platform-tests/shadow-dom/scroll-to-the-fragment-in-shadow-tree.html
fast/visual-viewport/zoomed-scroll-into-view-fixed.html
fast/scrolling/scroll-to-anchor-zoomed-header.html
Comment 5 Build Bot 2019-04-23 15:33:02 PDT
Created attachment 368071 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 6 Build Bot 2019-04-23 16:26:57 PDT
Comment on attachment 368063 [details]
Fixes the bug

Attachment 368063 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11975991

New failing tests:
fast/scrolling/programmatic-document-rtl-scrollIntoView.html
fast/dynamic/anchor-lock.html
fast/transforms/selection-bounds-in-transformed-view.html
imported/w3c/web-platform-tests/shadow-dom/scroll-to-the-fragment-in-shadow-tree.html
fast/visual-viewport/zoomed-scroll-into-view-fixed.html
fast/scrolling/scroll-to-anchor-zoomed-header.html
Comment 7 Build Bot 2019-04-23 16:26:58 PDT
Created attachment 368080 [details]
Archive of layout-test-results from ews114 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 8 Build Bot 2019-04-23 16:27:03 PDT
Comment on attachment 368063 [details]
Fixes the bug

Attachment 368063 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11975885

New failing tests:
fast/dom/focus-contenteditable.html
fast/dynamic/anchor-lock.html
imported/w3c/web-platform-tests/css/cssom-view/elementFromPoint.html
fast/scrolling/ios/scroll-into-view-with-top-content-inset.html
fast/scrolling/programmatic-document-rtl-scrollIntoView.html
Comment 9 Build Bot 2019-04-23 16:27:05 PDT
Created attachment 368081 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 10 Build Bot 2019-04-23 17:26:58 PDT
Comment on attachment 368063 [details]
Fixes the bug

Attachment 368063 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11976734

New failing tests:
fast/transforms/selection-bounds-in-transformed-view.html
fast/visual-viewport/zoomed-scroll-into-view-fixed.html
fast/dynamic/anchor-lock.html
fast/scrolling/scroll-to-anchor-zoomed-header.html
fast/scrolling/programmatic-document-rtl-scrollIntoView.html
Comment 11 Build Bot 2019-04-23 17:27:09 PDT
Created attachment 368088 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 12 Ryosuke Niwa 2019-04-23 18:56:14 PDT
Created attachment 368100 [details]
Fixes the bug
Comment 13 Simon Fraser (smfr) 2019-04-23 20:12:30 PDT
Comment on attachment 368100 [details]
Fixes the bug

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

> Source/WebCore/rendering/RenderLayer.cpp:2602
> +            IntRect snappedRevealRect = snappedIntRect(absoluteRect);
> +            snappedRevealRect.setLocation(clampedScrollPosition);

Remove these lines.

> Source/WebCore/rendering/RenderLayer.cpp:2604
>              frameView.setScrollPosition(roundedIntPoint(revealRect.location()));

Remove this line.
Comment 14 Ryosuke Niwa 2019-04-23 21:05:41 PDT
Created attachment 368108 [details]
Fixes the bug
Comment 15 Build Bot 2019-04-24 02:05:38 PDT
Comment on attachment 368108 [details]
Fixes the bug

Attachment 368108 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11980380

New failing tests:
fast/dom/focus-contenteditable.html
imported/w3c/web-platform-tests/css/cssom-view/elementFromPoint.html
fast/scrolling/ios/scroll-into-view-with-top-content-inset.html
Comment 16 Build Bot 2019-04-24 02:05:40 PDT
Created attachment 368113 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 17 Brent Fulgham 2019-04-26 12:41:06 PDT
I can't tell if the test failures related to this patch, but these failing test cases sound like they are the kind of things that would be impacted:

fast/dom/focus-contenteditable.html
fast/scrolling/ios/scroll-into-view-with-top-content-inset.html
Comment 18 Ryosuke Niwa 2019-04-30 20:36:41 PDT
Created attachment 368647 [details]
Fixed the test
Comment 19 Simon Fraser (smfr) 2019-05-01 14:31:04 PDT
Comment on attachment 368647 [details]
Fixed the test

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

> Source/WebCore/rendering/RenderLayer.cpp:2610
> +//            frameView.setScrollPosition(roundedIntPoint(revealRect.location()));

Remove this line.
Comment 20 Ryosuke Niwa 2019-05-01 14:38:03 PDT
Committed r244851: <https://trac.webkit.org/changeset/244851>