Bug 197211 - [iOS] Element::focus and Element::scrollIntoView do not clamp scroll positions
Summary: [iOS] Element::focus and Element::scrollIntoView do not clamp scroll positions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Scrolling (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-23 14:10 PDT by Ryosuke Niwa
Modified: 2019-05-02 15:44 PDT (History)
9 users (show)

See Also:


Attachments
Fixes the bug (10.69 KB, patch)
2019-04-23 14:21 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-highsierra (3.15 MB, application/zip)
2019-04-23 15:06 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (3.63 MB, application/zip)
2019-04-23 15:33 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-highsierra (3.07 MB, application/zip)
2019-04-23 16:26 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (4.34 MB, application/zip)
2019-04-23 16:27 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews200 for win-future (12.88 MB, application/zip)
2019-04-23 17:27 PDT, Build Bot
no flags Details
Fixes the bug (9.85 KB, patch)
2019-04-23 18:56 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Fixes the bug (9.79 KB, patch)
2019-04-23 21:05 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2 (2.72 MB, application/zip)
2019-04-24 02:05 PDT, Build Bot
no flags Details
Fixed the test (13.58 KB, patch)
2019-04-30 20:36 PDT, Ryosuke Niwa
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>