- Revision
- 249696
- Author
- alanc...@apple.com
- Date
- 2019-09-09 20:19:52 -0700 (Mon, 09 Sep 2019)
Log Message
Cherry-pick r249581. rdar://problem/55202922
REGRESSION (iOS 13): If an overflow:hidden with a non-zero scroll position is toggled to overflow:scroll, some other scroll causes its scroll position to get reset
https://bugs.webkit.org/show_bug.cgi?id=201528
rdar://problem/55044885
Reviewed by Frédéric Wang.
Source/WebCore:
If, when an overflow scrolling node is created, the scroller has non-zero scroll
position (for example, via toggling to overflow:hidden, setting scrollTop, then toggling
to overflow:scroll), then on the next update its scroll position will reset back to zero.
The bug was that newly created ScrollingTreeScrollingNodes didn't set m_currentScrollPosition
to the scroll position coming from the state node, so a subsequent update could cause
the 0,0 currentScrollPosition to get applied. If we're making a new node, and there's no
requestedScrollPosition, then initialize m_currentScrollPosition.
Test: scrollingcoordinator/ios/scroller-initial-scroll-position.html
* page/scrolling/ScrollingTreeScrollingNode.cpp:
(WebCore::ScrollingTreeScrollingNode::commitStateBeforeChildren):
(WebCore::ScrollingTreeScrollingNode::commitStateAfterChildren):
* page/scrolling/ScrollingTreeScrollingNode.h:
LayoutTests:
* scrollingcoordinator/ios/scroller-initial-scroll-position-expected.html: Added.
* scrollingcoordinator/ios/scroller-initial-scroll-position.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249581 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-608-branch/LayoutTests/ChangeLog (249695 => 249696)
--- branches/safari-608-branch/LayoutTests/ChangeLog 2019-09-10 03:19:48 UTC (rev 249695)
+++ branches/safari-608-branch/LayoutTests/ChangeLog 2019-09-10 03:19:52 UTC (rev 249696)
@@ -1,47 +1,81 @@
2019-09-09 Kocsen Chung <kocsen_ch...@apple.com>
- Cherry-pick r249565. rdar://problem/55113261
+ Cherry-pick r249581. rdar://problem/55202922
- AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
- https://bugs.webkit.org/show_bug.cgi?id=201518
- <rdar://problem/54835122>
+ REGRESSION (iOS 13): If an overflow:hidden with a non-zero scroll position is toggled to overflow:scroll, some other scroll causes its scroll position to get reset
+ https://bugs.webkit.org/show_bug.cgi?id=201528
+ rdar://problem/55044885
- Patch by Andres Gonzalez <andresg...@apple.com> on 2019-09-06
- Reviewed by Ryosuke Niwa.
-
+ Reviewed by Frédéric Wang.
Source/WebCore:
- Test: accessibility/set-selected-text-range-after-newline.html
+ If, when an overflow scrolling node is created, the scroller has non-zero scroll
+ position (for example, via toggling to overflow:hidden, setting scrollTop, then toggling
+ to overflow:scroll), then on the next update its scroll position will reset back to zero.
- In the case of an empty line, the CharacterIterator range start and end
- were not equal, thus we were not advancing the iterator and returning
- the iterator range end, which is not correct. With this change we are
- always advancing the iterator if its text is just '\n'. This covers all
- the cases we fixed before plus empty lines.
+ The bug was that newly created ScrollingTreeScrollingNodes didn't set m_currentScrollPosition
+ to the scroll position coming from the state node, so a subsequent update could cause
+ the 0,0 currentScrollPosition to get applied. If we're making a new node, and there's no
+ requestedScrollPosition, then initialize m_currentScrollPosition.
- * editing/Editing.cpp:
- (WebCore::visiblePositionForIndexUsingCharacterIterator):
+ Test: scrollingcoordinator/ios/scroller-initial-scroll-position.html
+ * page/scrolling/ScrollingTreeScrollingNode.cpp:
+ (WebCore::ScrollingTreeScrollingNode::commitStateBeforeChildren):
+ (WebCore::ScrollingTreeScrollingNode::commitStateAfterChildren):
+ * page/scrolling/ScrollingTreeScrollingNode.h:
+
LayoutTests:
- Extended this test to set the selection range passed an empty line.
- * accessibility/set-selected-text-range-after-newline-expected.txt:
- * accessibility/set-selected-text-range-after-newline.html:
+ * scrollingcoordinator/ios/scroller-initial-scroll-position-expected.html: Added.
+ * scrollingcoordinator/ios/scroller-initial-scroll-position.html: Added.
- git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249565 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249581 268f45cc-cd09-0410-ab3c-d52691b4dbfc
- 2019-09-06 Andres Gonzalez <andresg...@apple.com>
+ 2019-09-09 Kocsen Chung <kocsen_ch...@apple.com>
- AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
- https://bugs.webkit.org/show_bug.cgi?id=201518
- <rdar://problem/54835122>
+ Cherry-pick r249565. rdar://problem/55113261
- Reviewed by Ryosuke Niwa.
+ AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
+ https://bugs.webkit.org/show_bug.cgi?id=201518
+ <rdar://problem/54835122>
- Extended this test to set the selection range passed an empty line.
- * accessibility/set-selected-text-range-after-newline-expected.txt:
- * accessibility/set-selected-text-range-after-newline.html:
+ Patch by Andres Gonzalez <andresg...@apple.com> on 2019-09-06
+ Reviewed by Ryosuke Niwa.
+ Source/WebCore:
+
+ Test: accessibility/set-selected-text-range-after-newline.html
+
+ In the case of an empty line, the CharacterIterator range start and end
+ were not equal, thus we were not advancing the iterator and returning
+ the iterator range end, which is not correct. With this change we are
+ always advancing the iterator if its text is just '\n'. This covers all
+ the cases we fixed before plus empty lines.
+
+ * editing/Editing.cpp:
+ (WebCore::visiblePositionForIndexUsingCharacterIterator):
+
+ LayoutTests:
+
+ Extended this test to set the selection range passed an empty line.
+ * accessibility/set-selected-text-range-after-newline-expected.txt:
+ * accessibility/set-selected-text-range-after-newline.html:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249565 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-09-06 Andres Gonzalez <andresg...@apple.com>
+
+ AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
+ https://bugs.webkit.org/show_bug.cgi?id=201518
+ <rdar://problem/54835122>
+
+ Reviewed by Ryosuke Niwa.
+
+ Extended this test to set the selection range passed an empty line.
+ * accessibility/set-selected-text-range-after-newline-expected.txt:
+ * accessibility/set-selected-text-range-after-newline.html:
+
2019-09-09 Kocsen Chung <kocsen_ch...@apple.com>
Cherry-pick r249534. rdar://problem/55183098
@@ -630,6 +664,17 @@
(WebCore::FullscreenManager::clear):
(WebCore::FullscreenManager::fullscreenElementRemoved): Deleted.
* dom/FullscreenManager.h:
+2019-09-06 Simon Fraser <simon.fra...@apple.com>
+
+ REGRESSION (iOS 13): If an overflow:hidden with a non-zero scroll position is toggled to overflow:scroll, some other scroll causes its scroll position to get reset
+ https://bugs.webkit.org/show_bug.cgi?id=201528
+ rdar://problem/55044885
+
+ Reviewed by Frédéric Wang.
+
+ * scrollingcoordinator/ios/scroller-initial-scroll-position-expected.html: Added.
+ * scrollingcoordinator/ios/scroller-initial-scroll-position.html: Added.
+
Source/WebKit:
Added: branches/safari-608-branch/LayoutTests/scrollingcoordinator/ios/scroller-initial-scroll-position-expected.html (0 => 249696)
--- branches/safari-608-branch/LayoutTests/scrollingcoordinator/ios/scroller-initial-scroll-position-expected.html (rev 0)
+++ branches/safari-608-branch/LayoutTests/scrollingcoordinator/ios/scroller-initial-scroll-position-expected.html 2019-09-10 03:19:52 UTC (rev 249696)
@@ -0,0 +1,51 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+ <style>
+ body {
+ height: 4000px;
+ }
+
+ #scroller {
+ width: 300px;
+ height: 300px;
+ border: 1px solid black;
+ overflow-y: scroll;
+ -webkit-overflow-scrolling: touch;
+ }
+
+ .contents {
+ width: 100%;
+ height: 200%;
+ border-top: 200px solid red;
+ box-sizing: border-box;
+ background-color: green;
+ }
+
+ </style>
+ <script src=""
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ window.addEventListener('load', async () => {
+ scroller.scrollTop = 200;
+
+ if (!testRunner.runUIScript)
+ return
+
+ await UIHelper.immediateScrollTo(0, 10);
+ await UIHelper.ensurePresentationUpdate();
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, false);
+ </script>
+</head>
+<body>
+ <div id="scroller">
+ <div class="contents">
+ </div>
+ </div>
+</body>
+</html>
Added: branches/safari-608-branch/LayoutTests/scrollingcoordinator/ios/scroller-initial-scroll-position.html (0 => 249696)
--- branches/safari-608-branch/LayoutTests/scrollingcoordinator/ios/scroller-initial-scroll-position.html (rev 0)
+++ branches/safari-608-branch/LayoutTests/scrollingcoordinator/ios/scroller-initial-scroll-position.html 2019-09-10 03:19:52 UTC (rev 249696)
@@ -0,0 +1,53 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+ <style>
+ body {
+ height: 4000px;
+ }
+
+ #scroller {
+ width: 300px;
+ height: 300px;
+ border: 1px solid black;
+ overflow-y: scroll;
+ -webkit-overflow-scrolling: touch;
+ }
+
+ .contents {
+ width: 100%;
+ height: 200%;
+ border-top: 200px solid red;
+ box-sizing: border-box;
+ background-color: green;
+ }
+
+ </style>
+ <script src=""
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ window.addEventListener('load', async () => {
+ scroller.style.overflow = 'hidden';
+ scroller.scrollTop = 200;
+ scroller.style.overflow = 'scroll';
+
+ if (!testRunner.runUIScript)
+ return
+
+ await UIHelper.immediateScrollTo(0, 10);
+ await UIHelper.ensurePresentationUpdate();
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, false);
+ </script>
+</head>
+<body>
+ <div id="scroller">
+ <div class="contents">
+ </div>
+ </div>
+</body>
+</html>
Modified: branches/safari-608-branch/Source/WebCore/ChangeLog (249695 => 249696)
--- branches/safari-608-branch/Source/WebCore/ChangeLog 2019-09-10 03:19:48 UTC (rev 249695)
+++ branches/safari-608-branch/Source/WebCore/ChangeLog 2019-09-10 03:19:52 UTC (rev 249696)
@@ -1,54 +1,88 @@
2019-09-09 Kocsen Chung <kocsen_ch...@apple.com>
- Cherry-pick r249565. rdar://problem/55113261
+ Cherry-pick r249581. rdar://problem/55202922
- AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
- https://bugs.webkit.org/show_bug.cgi?id=201518
- <rdar://problem/54835122>
+ REGRESSION (iOS 13): If an overflow:hidden with a non-zero scroll position is toggled to overflow:scroll, some other scroll causes its scroll position to get reset
+ https://bugs.webkit.org/show_bug.cgi?id=201528
+ rdar://problem/55044885
- Patch by Andres Gonzalez <andresg...@apple.com> on 2019-09-06
- Reviewed by Ryosuke Niwa.
-
+ Reviewed by Frédéric Wang.
Source/WebCore:
- Test: accessibility/set-selected-text-range-after-newline.html
+ If, when an overflow scrolling node is created, the scroller has non-zero scroll
+ position (for example, via toggling to overflow:hidden, setting scrollTop, then toggling
+ to overflow:scroll), then on the next update its scroll position will reset back to zero.
- In the case of an empty line, the CharacterIterator range start and end
- were not equal, thus we were not advancing the iterator and returning
- the iterator range end, which is not correct. With this change we are
- always advancing the iterator if its text is just '\n'. This covers all
- the cases we fixed before plus empty lines.
+ The bug was that newly created ScrollingTreeScrollingNodes didn't set m_currentScrollPosition
+ to the scroll position coming from the state node, so a subsequent update could cause
+ the 0,0 currentScrollPosition to get applied. If we're making a new node, and there's no
+ requestedScrollPosition, then initialize m_currentScrollPosition.
- * editing/Editing.cpp:
- (WebCore::visiblePositionForIndexUsingCharacterIterator):
+ Test: scrollingcoordinator/ios/scroller-initial-scroll-position.html
+ * page/scrolling/ScrollingTreeScrollingNode.cpp:
+ (WebCore::ScrollingTreeScrollingNode::commitStateBeforeChildren):
+ (WebCore::ScrollingTreeScrollingNode::commitStateAfterChildren):
+ * page/scrolling/ScrollingTreeScrollingNode.h:
+
LayoutTests:
- Extended this test to set the selection range passed an empty line.
- * accessibility/set-selected-text-range-after-newline-expected.txt:
- * accessibility/set-selected-text-range-after-newline.html:
+ * scrollingcoordinator/ios/scroller-initial-scroll-position-expected.html: Added.
+ * scrollingcoordinator/ios/scroller-initial-scroll-position.html: Added.
- git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249565 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249581 268f45cc-cd09-0410-ab3c-d52691b4dbfc
- 2019-09-06 Andres Gonzalez <andresg...@apple.com>
+ 2019-09-09 Kocsen Chung <kocsen_ch...@apple.com>
- AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
- https://bugs.webkit.org/show_bug.cgi?id=201518
- <rdar://problem/54835122>
+ Cherry-pick r249565. rdar://problem/55113261
- Reviewed by Ryosuke Niwa.
+ AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
+ https://bugs.webkit.org/show_bug.cgi?id=201518
+ <rdar://problem/54835122>
- Test: accessibility/set-selected-text-range-after-newline.html
+ Patch by Andres Gonzalez <andresg...@apple.com> on 2019-09-06
+ Reviewed by Ryosuke Niwa.
- In the case of an empty line, the CharacterIterator range start and end
- were not equal, thus we were not advancing the iterator and returning
- the iterator range end, which is not correct. With this change we are
- always advancing the iterator if its text is just '\n'. This covers all
- the cases we fixed before plus empty lines.
+ Source/WebCore:
- * editing/Editing.cpp:
- (WebCore::visiblePositionForIndexUsingCharacterIterator):
+ Test: accessibility/set-selected-text-range-after-newline.html
+ In the case of an empty line, the CharacterIterator range start and end
+ were not equal, thus we were not advancing the iterator and returning
+ the iterator range end, which is not correct. With this change we are
+ always advancing the iterator if its text is just '\n'. This covers all
+ the cases we fixed before plus empty lines.
+
+ * editing/Editing.cpp:
+ (WebCore::visiblePositionForIndexUsingCharacterIterator):
+
+ LayoutTests:
+
+ Extended this test to set the selection range passed an empty line.
+ * accessibility/set-selected-text-range-after-newline-expected.txt:
+ * accessibility/set-selected-text-range-after-newline.html:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249565 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-09-06 Andres Gonzalez <andresg...@apple.com>
+
+ AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
+ https://bugs.webkit.org/show_bug.cgi?id=201518
+ <rdar://problem/54835122>
+
+ Reviewed by Ryosuke Niwa.
+
+ Test: accessibility/set-selected-text-range-after-newline.html
+
+ In the case of an empty line, the CharacterIterator range start and end
+ were not equal, thus we were not advancing the iterator and returning
+ the iterator range end, which is not correct. With this change we are
+ always advancing the iterator if its text is just '\n'. This covers all
+ the cases we fixed before plus empty lines.
+
+ * editing/Editing.cpp:
+ (WebCore::visiblePositionForIndexUsingCharacterIterator):
+
2019-09-09 Kocsen Chung <kocsen_ch...@apple.com>
Cherry-pick r249534. rdar://problem/55183098
@@ -511,6 +545,30 @@
conversion for 8-bit strings.
Tools:
+2019-09-06 Simon Fraser <simon.fra...@apple.com>
+
+ REGRESSION (iOS 13): If an overflow:hidden with a non-zero scroll position is toggled to overflow:scroll, some other scroll causes its scroll position to get reset
+ https://bugs.webkit.org/show_bug.cgi?id=201528
+ rdar://problem/55044885
+
+ Reviewed by Frédéric Wang.
+
+ If, when an overflow scrolling node is created, the scroller has non-zero scroll
+ position (for example, via toggling to overflow:hidden, setting scrollTop, then toggling
+ to overflow:scroll), then on the next update its scroll position will reset back to zero.
+
+ The bug was that newly created ScrollingTreeScrollingNodes didn't set m_currentScrollPosition
+ to the scroll position coming from the state node, so a subsequent update could cause
+ the 0,0 currentScrollPosition to get applied. If we're making a new node, and there's no
+ requestedScrollPosition, then initialize m_currentScrollPosition.
+
+ Test: scrollingcoordinator/ios/scroller-initial-scroll-position.html
+
+ * page/scrolling/ScrollingTreeScrollingNode.cpp:
+ (WebCore::ScrollingTreeScrollingNode::commitStateBeforeChildren):
+ (WebCore::ScrollingTreeScrollingNode::commitStateAfterChildren):
+ * page/scrolling/ScrollingTreeScrollingNode.h:
+
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: Add
DataDetectorsTestIOS.mm to the project.
Modified: branches/safari-608-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp (249695 => 249696)
--- branches/safari-608-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2019-09-10 03:19:48 UTC (rev 249695)
+++ branches/safari-608-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2019-09-10 03:19:52 UTC (rev 249696)
@@ -62,8 +62,11 @@
if (state.hasChangedProperty(ScrollingStateScrollingNode::ReachableContentsSize))
m_reachableContentsSize = state.reachableContentsSize();
- if (state.hasChangedProperty(ScrollingStateScrollingNode::ScrollPosition))
+ if (state.hasChangedProperty(ScrollingStateScrollingNode::ScrollPosition)) {
m_lastCommittedScrollPosition = state.scrollPosition();
+ if (m_isFirstCommit && !state.hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition))
+ m_currentScrollPosition = m_lastCommittedScrollPosition;
+ }
if (state.hasChangedProperty(ScrollingStateScrollingNode::ParentRelativeScrollableRect))
m_parentRelativeScrollableRect = state.parentRelativeScrollableRect();
@@ -111,6 +114,8 @@
const ScrollingStateScrollingNode& scrollingStateNode = downcast<ScrollingStateScrollingNode>(stateNode);
if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition))
scrollingTree().scrollingTreeNodeRequestsScroll(scrollingNodeID(), scrollingStateNode.requestedScrollPosition(), scrollingStateNode.requestedScrollPositionRepresentsProgrammaticScroll());
+
+ m_isFirstCommit = false;
}
ScrollingEventResult ScrollingTreeScrollingNode::handleWheelEvent(const PlatformWheelEvent&)
Modified: branches/safari-608-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h (249695 => 249696)
--- branches/safari-608-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h 2019-09-10 03:19:48 UTC (rev 249695)
+++ branches/safari-608-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h 2019-09-10 03:19:52 UTC (rev 249696)
@@ -151,6 +151,7 @@
#endif
ScrollableAreaParameters m_scrollableAreaParameters;
bool m_expectsWheelEventTestTrigger { false };
+ bool m_isFirstCommit { true };
#if PLATFORM(COCOA)
RetainPtr<CALayer> m_scrollContainerLayer;
