⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 249696 in webkit


Ignore:
Timestamp:
Sep 9, 2019, 8:19:52 PM (7 years ago)
Author:
Alan Coon
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

Location:
branches/safari-608-branch
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-608-branch/LayoutTests/ChangeLog

    r249695 r249696  
    112019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
    22
    3         Cherry-pick r249565. rdar://problem/55113261
    4 
    5     AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
    6     https://bugs.webkit.org/show_bug.cgi?id=201518
    7     <rdar://problem/54835122>
    8    
    9     Patch by Andres Gonzalez <andresg_22@apple.com> on 2019-09-06
    10     Reviewed by Ryosuke Niwa.
    11    
     3        Cherry-pick r249581. rdar://problem/55202922
     4
     5    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
     6    https://bugs.webkit.org/show_bug.cgi?id=201528
     7    rdar://problem/55044885
     8   
     9    Reviewed by Frédéric Wang.
    1210    Source/WebCore:
    1311   
    14     Test: accessibility/set-selected-text-range-after-newline.html
    15    
    16     In the case of an empty line, the CharacterIterator range start and end
    17     were not equal, thus we were not advancing the iterator and returning
    18     the iterator range end, which is not correct. With this change we are
    19     always advancing the iterator if its text is just '\n'. This covers all
    20     the cases we fixed before plus empty lines.
    21    
    22     * editing/Editing.cpp:
    23     (WebCore::visiblePositionForIndexUsingCharacterIterator):
     12    If, when an overflow scrolling node is created, the scroller has non-zero scroll
     13    position (for example, via toggling to overflow:hidden, setting scrollTop, then toggling
     14    to overflow:scroll), then on the next update its scroll position will reset back to zero.
     15   
     16    The bug was that newly created ScrollingTreeScrollingNodes didn't set m_currentScrollPosition
     17    to the scroll position coming from the state node, so a subsequent update could cause
     18    the 0,0 currentScrollPosition to get applied. If we're making a new node, and there's no
     19    requestedScrollPosition, then initialize m_currentScrollPosition.
     20   
     21    Test: scrollingcoordinator/ios/scroller-initial-scroll-position.html
     22   
     23    * page/scrolling/ScrollingTreeScrollingNode.cpp:
     24    (WebCore::ScrollingTreeScrollingNode::commitStateBeforeChildren):
     25    (WebCore::ScrollingTreeScrollingNode::commitStateAfterChildren):
     26    * page/scrolling/ScrollingTreeScrollingNode.h:
    2427   
    2528    LayoutTests:
    2629   
    27     Extended this test to set the selection range passed an empty line.
    28     * accessibility/set-selected-text-range-after-newline-expected.txt:
    29     * accessibility/set-selected-text-range-after-newline.html:
    30    
    31     git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249565 268f45cc-cd09-0410-ab3c-d52691b4dbfc
    32 
    33     2019-09-06  Andres Gonzalez  <andresg_22@apple.com>
    34 
    35             AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
    36             https://bugs.webkit.org/show_bug.cgi?id=201518
    37             <rdar://problem/54835122>
    38 
    39             Reviewed by Ryosuke Niwa.
    40 
    41             Extended this test to set the selection range passed an empty line.
    42             * accessibility/set-selected-text-range-after-newline-expected.txt:
    43             * accessibility/set-selected-text-range-after-newline.html:
     30    * scrollingcoordinator/ios/scroller-initial-scroll-position-expected.html: Added.
     31    * scrollingcoordinator/ios/scroller-initial-scroll-position.html: Added.
     32   
     33    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249581 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     34
     35    2019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
     36
     37            Cherry-pick r249565. rdar://problem/55113261
     38
     39        AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
     40        https://bugs.webkit.org/show_bug.cgi?id=201518
     41        <rdar://problem/54835122>
     42
     43        Patch by Andres Gonzalez <andresg_22@apple.com> on 2019-09-06
     44        Reviewed by Ryosuke Niwa.
     45
     46        Source/WebCore:
     47
     48        Test: accessibility/set-selected-text-range-after-newline.html
     49
     50        In the case of an empty line, the CharacterIterator range start and end
     51        were not equal, thus we were not advancing the iterator and returning
     52        the iterator range end, which is not correct. With this change we are
     53        always advancing the iterator if its text is just '\n'. This covers all
     54        the cases we fixed before plus empty lines.
     55
     56        * editing/Editing.cpp:
     57        (WebCore::visiblePositionForIndexUsingCharacterIterator):
     58
     59        LayoutTests:
     60
     61        Extended this test to set the selection range passed an empty line.
     62        * accessibility/set-selected-text-range-after-newline-expected.txt:
     63        * accessibility/set-selected-text-range-after-newline.html:
     64
     65        git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249565 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     66
     67        2019-09-06  Andres Gonzalez  <andresg_22@apple.com>
     68
     69                AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
     70                https://bugs.webkit.org/show_bug.cgi?id=201518
     71                <rdar://problem/54835122>
     72
     73                Reviewed by Ryosuke Niwa.
     74
     75                Extended this test to set the selection range passed an empty line.
     76                * accessibility/set-selected-text-range-after-newline-expected.txt:
     77                * accessibility/set-selected-text-range-after-newline.html:
    4478
    45792019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
    … …  
    631665    (WebCore::FullscreenManager::fullscreenElementRemoved): Deleted.
    632666    * dom/FullscreenManager.h:
     6672019-09-06  Simon Fraser  <simon.fraser@apple.com>
     668
     669        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
     670        https://bugs.webkit.org/show_bug.cgi?id=201528
     671        rdar://problem/55044885
     672
     673        Reviewed by Frédéric Wang.
     674
     675        * scrollingcoordinator/ios/scroller-initial-scroll-position-expected.html: Added.
     676        * scrollingcoordinator/ios/scroller-initial-scroll-position.html: Added.
     677
    633678   
    634679    Source/WebKit:

  • branches/safari-608-branch/Source/WebCore/ChangeLog

    r249695 r249696  
    112019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
    22
    3         Cherry-pick r249565. rdar://problem/55113261
    4 
    5     AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
    6     https://bugs.webkit.org/show_bug.cgi?id=201518
    7     <rdar://problem/54835122>
    8    
    9     Patch by Andres Gonzalez <andresg_22@apple.com> on 2019-09-06
    10     Reviewed by Ryosuke Niwa.
    11    
     3        Cherry-pick r249581. rdar://problem/55202922
     4
     5    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
     6    https://bugs.webkit.org/show_bug.cgi?id=201528
     7    rdar://problem/55044885
     8   
     9    Reviewed by Frédéric Wang.
    1210    Source/WebCore:
    1311   
    14     Test: accessibility/set-selected-text-range-after-newline.html
    15    
    16     In the case of an empty line, the CharacterIterator range start and end
    17     were not equal, thus we were not advancing the iterator and returning
    18     the iterator range end, which is not correct. With this change we are
    19     always advancing the iterator if its text is just '\n'. This covers all
    20     the cases we fixed before plus empty lines.
    21    
    22     * editing/Editing.cpp:
    23     (WebCore::visiblePositionForIndexUsingCharacterIterator):
     12    If, when an overflow scrolling node is created, the scroller has non-zero scroll
     13    position (for example, via toggling to overflow:hidden, setting scrollTop, then toggling
     14    to overflow:scroll), then on the next update its scroll position will reset back to zero.
     15   
     16    The bug was that newly created ScrollingTreeScrollingNodes didn't set m_currentScrollPosition
     17    to the scroll position coming from the state node, so a subsequent update could cause
     18    the 0,0 currentScrollPosition to get applied. If we're making a new node, and there's no
     19    requestedScrollPosition, then initialize m_currentScrollPosition.
     20   
     21    Test: scrollingcoordinator/ios/scroller-initial-scroll-position.html
     22   
     23    * page/scrolling/ScrollingTreeScrollingNode.cpp:
     24    (WebCore::ScrollingTreeScrollingNode::commitStateBeforeChildren):
     25    (WebCore::ScrollingTreeScrollingNode::commitStateAfterChildren):
     26    * page/scrolling/ScrollingTreeScrollingNode.h:
    2427   
    2528    LayoutTests:
    2629   
    27     Extended this test to set the selection range passed an empty line.
    28     * accessibility/set-selected-text-range-after-newline-expected.txt:
    29     * accessibility/set-selected-text-range-after-newline.html:
    30    
    31     git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249565 268f45cc-cd09-0410-ab3c-d52691b4dbfc
    32 
    33     2019-09-06  Andres Gonzalez  <andresg_22@apple.com>
    34 
    35             AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
    36             https://bugs.webkit.org/show_bug.cgi?id=201518
    37             <rdar://problem/54835122>
    38 
    39             Reviewed by Ryosuke Niwa.
    40 
    41             Test: accessibility/set-selected-text-range-after-newline.html
    42 
    43             In the case of an empty line, the CharacterIterator range start and end
    44             were not equal, thus we were not advancing the iterator and returning
    45             the iterator range end, which is not correct. With this change we are
    46             always advancing the iterator if its text is just '\n'. This covers all
    47             the cases we fixed before plus empty lines.
    48 
    49             * editing/Editing.cpp:
    50             (WebCore::visiblePositionForIndexUsingCharacterIterator):
     30    * scrollingcoordinator/ios/scroller-initial-scroll-position-expected.html: Added.
     31    * scrollingcoordinator/ios/scroller-initial-scroll-position.html: Added.
     32   
     33    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249581 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     34
     35    2019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
     36
     37            Cherry-pick r249565. rdar://problem/55113261
     38
     39        AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
     40        https://bugs.webkit.org/show_bug.cgi?id=201518
     41        <rdar://problem/54835122>
     42
     43        Patch by Andres Gonzalez <andresg_22@apple.com> on 2019-09-06
     44        Reviewed by Ryosuke Niwa.
     45
     46        Source/WebCore:
     47
     48        Test: accessibility/set-selected-text-range-after-newline.html
     49
     50        In the case of an empty line, the CharacterIterator range start and end
     51        were not equal, thus we were not advancing the iterator and returning
     52        the iterator range end, which is not correct. With this change we are
     53        always advancing the iterator if its text is just '\n'. This covers all
     54        the cases we fixed before plus empty lines.
     55
     56        * editing/Editing.cpp:
     57        (WebCore::visiblePositionForIndexUsingCharacterIterator):
     58
     59        LayoutTests:
     60
     61        Extended this test to set the selection range passed an empty line.
     62        * accessibility/set-selected-text-range-after-newline-expected.txt:
     63        * accessibility/set-selected-text-range-after-newline.html:
     64
     65        git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249565 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     66
     67        2019-09-06  Andres Gonzalez  <andresg_22@apple.com>
     68
     69                AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
     70                https://bugs.webkit.org/show_bug.cgi?id=201518
     71                <rdar://problem/54835122>
     72
     73                Reviewed by Ryosuke Niwa.
     74
     75                Test: accessibility/set-selected-text-range-after-newline.html
     76
     77                In the case of an empty line, the CharacterIterator range start and end
     78                were not equal, thus we were not advancing the iterator and returning
     79                the iterator range end, which is not correct. With this change we are
     80                always advancing the iterator if its text is just '\n'. This covers all
     81                the cases we fixed before plus empty lines.
     82
     83                * editing/Editing.cpp:
     84                (WebCore::visiblePositionForIndexUsingCharacterIterator):
    5185
    52862019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
    … …  
    512546   
    513547    Tools:
     5482019-09-06  Simon Fraser  <simon.fraser@apple.com>
     549
     550        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
     551        https://bugs.webkit.org/show_bug.cgi?id=201528
     552        rdar://problem/55044885
     553
     554        Reviewed by Frédéric Wang.
     555       
     556        If, when an overflow scrolling node is created, the scroller has non-zero scroll
     557        position (for example, via toggling to overflow:hidden, setting scrollTop, then toggling
     558        to overflow:scroll), then on the next update its scroll position will reset back to zero.
     559
     560        The bug was that newly created ScrollingTreeScrollingNodes didn't set m_currentScrollPosition
     561        to the scroll position coming from the state node, so a subsequent update could cause
     562        the 0,0 currentScrollPosition to get applied. If we're making a new node, and there's no
     563        requestedScrollPosition, then initialize m_currentScrollPosition.
     564
     565        Test: scrollingcoordinator/ios/scroller-initial-scroll-position.html
     566
     567        * page/scrolling/ScrollingTreeScrollingNode.cpp:
     568        (WebCore::ScrollingTreeScrollingNode::commitStateBeforeChildren):
     569        (WebCore::ScrollingTreeScrollingNode::commitStateAfterChildren):
     570        * page/scrolling/ScrollingTreeScrollingNode.h:
     571
    514572   
    515573    * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: Add

  • branches/safari-608-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp

    r247013 r249696  
    6363        m_reachableContentsSize = state.reachableContentsSize();
    6464
    65     if (state.hasChangedProperty(ScrollingStateScrollingNode::ScrollPosition))
     65    if (state.hasChangedProperty(ScrollingStateScrollingNode::ScrollPosition)) {
    6666        m_lastCommittedScrollPosition = state.scrollPosition();
     67        if (m_isFirstCommit && !state.hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition))
     68            m_currentScrollPosition = m_lastCommittedScrollPosition;
     69    }
    6770
    6871    if (state.hasChangedProperty(ScrollingStateScrollingNode::ParentRelativeScrollableRect))
    … …  
    112115    if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition))
    113116        scrollingTree().scrollingTreeNodeRequestsScroll(scrollingNodeID(), scrollingStateNode.requestedScrollPosition(), scrollingStateNode.requestedScrollPositionRepresentsProgrammaticScroll());
     117
     118    m_isFirstCommit = false;
    114119}
    115120

  • branches/safari-608-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h

    r247013 r249696  
    152152    ScrollableAreaParameters m_scrollableAreaParameters;
    153153    bool m_expectsWheelEventTestTrigger { false };
     154    bool m_isFirstCommit { true };
    154155
    155156#if PLATFORM(COCOA)
Note: See TracChangeset for help on using the changeset viewer.