Bug 196337 – Scroll position gets reset when overflow:scroll is inside grid

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Chris]

Created attachment 366147 [details]
Example Video

With the release of 12.1, we began getting reports of strange scrolling behavior.  

This JSFiddle should demonstrate the issue:

https://jsfiddle.net/blackoctopus0/7f56qx9c/2/

Hovering over a "message" in the example above will utilize the following CSS style:  

.message:hover .message-span {
  display: block;
}

Doing so causes the scrollable div to scroll up.  This is undesired.  Utilizing a framework like Vue can also cause this.  Utilizing v-show.  This function toggles elements using display: block/none;

Please feel free to contact me with further questions.  See attached video from the above fiddle.

Chris

[Radar WebKit Bug Importer]

<rdar://problem/49385784>

[Simon Fraser (smfr)]

This is the bug where updateScrollInfoAfterLayout() clobbers scroll position:

(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 7.1
    frame #0: 0x000000052ce951ea WebCore`WebCore::RenderLayer::scrollTo(this=0x000000056cae1e70, position={ x = 0, y = 0 }) at RenderLayer.cpp:2354:5
    frame #1: 0x000000052ce98b63 WebCore`WebCore::RenderLayer::setScrollOffset(this=0x000000056cae1e70, offset={ x = 0, y = 0 }) at RenderLayer.cpp:2787:5
    frame #2: 0x000000052c89a93e WebCore`WebCore::ScrollableArea::scrollPositionChanged(this=0x000000056cae1e70, position={ x = 0, y = 0 }) at ScrollableArea.cpp:166:5
    frame #3: 0x000000052c89ac98 WebCore`WebCore::ScrollableArea::setScrollOffsetFromAnimation(this=0x000000056cae1e70, offset={ x = 0, y = 0 }) at ScrollableArea.cpp:229:5
    frame #4: 0x000000052c88f536 WebCore`WebCore::ScrollAnimator::notifyPositionChanged(this=0x00000005712c5370, delta={ width = 0.0, height = -179.0 }) at ScrollAnimator.cpp:193:22
    frame #5: 0x0000000529b211a3 WebCore`WebCore::ScrollAnimatorMac::notifyPositionChanged(this=0x00000005712c5370, delta={ width = 0.0, height = -179.0 }) at ScrollAnimatorMac.mm:858:21
    frame #6: 0x0000000529b20c8c WebCore`WebCore::ScrollAnimatorMac::immediateScrollToPosition(this=0x00000005712c5370, newPosition={ x = 0.0, y = 0.0 }, clamping=Clamped) at ScrollAnimatorMac.mm:827:5
    frame #7: 0x0000000529b20bb3 WebCore`WebCore::ScrollAnimatorMac::scrollToOffsetWithoutAnimation(this=0x00000005712c5370, offset={ x = 0.0, y = 0.0 }, clamping=Clamped) at ScrollAnimatorMac.mm:793:5
    frame #8: 0x000000052c895f77 WebCore`WebCore::ScrollableArea::scrollToOffsetWithoutAnimation(this=0x000000056cae1e70, offset={ x = 0.0, y = 0.0 }, clamping=Clamped) at ScrollableArea.cpp:144:22
    frame #9: 0x000000052ce94ef5 WebCore`WebCore::RenderLayer::scrollToOffset(this=0x000000056cae1e70, scrollOffset={ x = 0, y = 0 }, clamping=Clamped) at RenderLayer.cpp:2345:9
    frame #10: 0x000000052ce9cb1d WebCore`WebCore::RenderLayer::updateScrollInfoAfterLayout(this=0x000000056cae1e70) at RenderLayer.cpp:3583:13
  * frame #11: 0x000000052cd28a0e WebCore`WebCore::RenderBlock::updateScrollInfoAfterLayout(this=0x000000054c3015f0) at RenderBlock.cpp:588:18
    frame #12: 0x000000052ce1ad30 WebCore`WebCore::RenderFlexibleBox::layoutBlock(this=0x000000054c3015f0, relayoutChildren=true, (null)={ 0px (0) }) at RenderFlexibleBox.cpp:318:5
    frame #13: 0x000000052cd28adc WebCore`WebCore::RenderBlock::layout(this=0x000000054c3015f0) at RenderBlock.cpp:598:5
    frame #14: 0x000000052ccc575c WebCore`WebCore::RenderElement::layoutIfNeeded(this=0x000000054c3015f0) at RenderElement.h:122:48
    frame #15: 0x000000052ccc764b WebCore`WebCore::DefiniteSizeStrategy::layoutGridItemForMinSizeComputation(this=0x000000057120f020, child=0x000000054c3015f0, overrideSizeHasChanged=false) const at GridTrackSizingAlgorithm.cpp:1070:11
    frame #16: 0x000000052ccc36cb WebCore`WebCore::GridTrackSizingAlgorithmStrategy::minSizeForChild(this=0x000000057120f020, child=0x000000054c3015f0) const at GridTrackSizingAlgorithm.cpp:832:5
    frame #17: 0x000000052ccc27a8 WebCore`WebCore::GridTrackSizingAlgorithm::sizeTrackToFitNonSpanningItem(this=0x000000054d500c48, span=0x00007ffee1e12228, gridItem=0x000000054c3015f0, track=0x000000056fb598dc) at GridTrackSizingAlgorithm.cpp:249:66
    frame #18: 0x000000052ccc7f4d WebCore`WebCore::GridTrackSizingAlgorithm::resolveIntrinsicTrackSizes(this=0x000000054d500c48) at GridTrackSizingAlgorithm.cpp:1144:25
    frame #19: 0x000000052cccb75a WebCore`WebCore::GridTrackSizingAlgorithm::run(this=0x000000054d500c48) at GridTrackSizingAlgorithm.cpp:1313:9
    frame #20: 0x000000052ce587be WebCore`WebCore::RenderGrid::computeTrackSizesForDefiniteSize(this=0x000000054d500a98, direction=ForRows, availableSpace={ 533px (34112) }) at RenderGrid.cpp:146:28
    frame #21: 0x000000052ce59993 WebCore`WebCore::RenderGrid::layoutBlock(this=0x000000054d500a98, relayoutChildren=false, (null)={ 0px (0) }) at RenderGrid.cpp:251:13
    frame #22: 0x000000052cd28adc WebCore`WebCore::RenderBlock::layout(this=0x000000054d500a98) at RenderBlock.cpp:598:5
    frame #23: 0x000000052cd43382 WebCore`WebCore::RenderBlockFlow::layoutBlockChild(this=0x000000056d502e20, child=0x000000054d500a98, marginInfo=0x00007ffee1e128c8, previousFloatLogicalBottom={ 0px (0) }, maxFloatLogicalBottom={ 0px (0) }) at RenderBlockFlow.cpp:730:15
    frame #24: 0x000000052cd41c84 WebCore`WebCore::RenderBlockFlow::layoutBlockChildren(this=0x000000056d502e20, relayoutChildren=false, maxFloatLogicalBottom={ 0px (0) }) at RenderBlockFlow.cpp:653:9
    frame #25: 0x000000052cd40b32 WebCore`WebCore::RenderBlockFlow::layoutBlock(this=0x000000056d502e20, relayoutChildren=false, pageLogicalHeight={ 0px (0) }) at RenderBlockFlow.cpp:505:13
    frame #26: 0x000000052cd28adc WebCore`WebCore::RenderBlock::layout(this=0x000000056d502e20) at RenderBlock.cpp:598:5
    frame #27: 0x000000052cd43382 WebCore`WebCore::RenderBlockFlow::layoutBlockChild(this=0x000000056d501680, child=0x000000056d502e20, marginInfo=0x00007ffee1e12ef8, previousFloatLogicalBottom={ 8px (512) }, maxFloatLogicalBottom={ 0px (0) }) at RenderBlockFlow.cpp:730:15
    frame #28: 0x000000052cd41c84 WebCore`WebCore::RenderBlockFlow::layoutBlockChildren(this=0x000000056d501680, relayoutChildren=false, maxFloatLogicalBottom={ 0px (0) }) at RenderBlockFlow.cpp:653:9
    frame #29: 0x000000052cd40b32 WebCore`WebCore::RenderBlockFlow::layoutBlock(this=0x000000056d501680, relayoutChildren=false, pageLogicalHeight={ 0px (0) }) at RenderBlockFlow.cpp:505:13
    frame #30: 0x000000052cd28adc WebCore`WebCore::RenderBlock::layout(this=0x000000056d501680) at RenderBlock.cpp:598:5
    frame #31: 0x000000052cd43382 WebCore`WebCore::RenderBlockFlow::layoutBlockChild(this=0x000000054ae00b20, child=0x000000056d501680, marginInfo=0x00007ffee1e13528, previousFloatLogicalBottom={ 0px (0) }, maxFloatLogicalBottom={ 0px (0) }) at RenderBlockFlow.cpp:730:15
    frame #32: 0x000000052cd41c84 WebCore`WebCore::RenderBlockFlow::layoutBlockChildren(this=0x000000054ae00b20, relayoutChildren=false, maxFloatLogicalBottom={ 0px (0) }) at RenderBlockFlow.cpp:653:9
    frame #33: 0x000000052cd40b32 WebCore`WebCore::RenderBlockFlow::layoutBlock(this=0x000000054ae00b20, relayoutChildren=false, pageLogicalHeight={ 0px (0) }) at RenderBlockFlow.cpp:505:13
    frame #34: 0x000000052cd28adc WebCore`WebCore::RenderBlock::layout(this=0x000000054ae00b20) at RenderBlock.cpp:598:5
    frame #35: 0x000000052cfcf6d8 WebCore`WebCore::RenderView::layout(this=0x000000054ae00b20) at RenderView.cpp:241:22
    frame #36: 0x000000052c6d56fc WebCore`WebCore::FrameViewLayoutContext::layout(this=0x0000000548f011e8) at FrameViewLayoutContext.cpp:212:21
    frame #37: 0x000000052bbe0cb6 WebCore`WebCore::Document::updateLayout(this={ origin = https://fiddle.jshell.net, url = https://fiddle.jshell.net/blackoctopus0/7f56qx9c/2/show/, inMainFrame = 0, pageCacheState = NotInPageCache }) at Document.cpp:2070:36
    frame #38: 0x000000052cfced16 WebCore`WebCore::RenderView::hitTest(this=0x000000054ae00b20, request=0x00007ffee1e140e8, location=0x00007ffee1e13da8, result=0x00007ffee1e13da8) at RenderView.cpp:147:16
    frame #39: 0x000000052cfcecd4 WebCore`WebCore::RenderView::hitTest(this=0x000000054ae00b20, request=0x00007ffee1e140e8, result=0x00007ffee1e13da8) at RenderView.cpp:142:12
    frame #40: 0x000000052bbf002b WebCore`WebCore::Document::prepareMouseEvent(this={ origin = https://fiddle.jshell.net, url = https://fiddle.jshell.net/blackoctopus0/7f56qx9c/2/show/, inMainFrame = 0, pageCacheState = NotInPageCache }, request=0x00007ffee1e140e8, documentPoint={ x = -776px (-49664), y = 510px (32640) }, event=0x00007ffee1e14250) at Document.cpp:3746:19
    frame #41: 0x000000052c67d004 WebCore`WebCore::EventHandler::prepareMouseEvent(this=0x0000000548051080, request=0x00007ffee1e140e8, mouseEvent=0x00007ffee1e14250) at EventHandler.cpp:2467:32
    frame #42: 0x000000052c67d6bb WebCore`WebCore::EventHandler::handleMouseMoveEvent(this=0x0000000548051080, platformMouseEvent=0x00007ffee1e14250, hoveredNode=0x0000000000000000, onlyUpdateScrollbars=false) at EventHandler.cpp:1964:47
    frame #43: 0x0000000529d16b04 WebCore`WebCore::EventHandler::passMouseMoveEventToSubframe(this=0x00000005480fa000, mev=0x00007ffee1e14250, subframe={ origin = https://fiddle.jshell.net, url = https://fiddle.jshell.net/blackoctopus0/7f56qx9c/2/show/, isMainFrame = 0, pageCacheState = NotInPageCache }, hoveredNode=0x0000000000000000) at EventHandlerMac.mm:701:30
    frame #44: 0x000000052c67d93a WebCore`WebCore::EventHandler::handleMouseMoveEvent(this=0x00000005480fa000, platformMouseEvent=0x00007ffee1e14500, hoveredNode=0x00007ffee1e143d8, onlyUpdateScrollbars=false) at EventHandler.cpp:1992:9
    frame #45: 0x000000052c67d1b3 WebCore`WebCore::EventHandler::mouseMoved(this=0x00000005480fa000, event=0x00007ffee1e14500) at EventHandler.cpp:1873:19

We have other bugs about this (can't find them).

[Simon Fraser (smfr)]

I think this bug may be grid-specific. See also bug 193769

[Simon Fraser (smfr)]

Probably need some beginUpdateScrollInfoAfterLayoutTransaction somewhere.

[Manuel Rego Casasnovas]

Created attachment 366264 [details]
Patch

[Manuel Rego Casasnovas]

(In reply to Simon Fraser (smfr) from comment #4)
> Probably need some beginUpdateScrollInfoAfterLayoutTransaction somewhere.

We were not using that on RenderGrid.
Adding that together with endAndCommitUpdateScrollInfoAfterLayoutTransaction fixes the issue.
Thanks for the pointer.

[WebKit Commit Bot]

Comment on attachment 366264 [details]
Patch

Clearing flags on attachment: 366264

Committed r243687: <https://trac.webkit.org/changeset/243687>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Manuel Rego Casasnovas]

*** Bug 191506 has been marked as a duplicate of this bug. ***