Bug 169900 - RTL: scrolling a div with 'overflow-x: hidden' causes content to overflow to the right instead of left
Summary: RTL: scrolling a div with 'overflow-x: hidden' causes content to overflow to ...
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: BJ Burg
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-03-20 17:21 PDT by BJ Burg
Modified: 2021-07-07 16:22 PDT (History)
7 users (show)

See Also:


Attachments
reduction (371 bytes, text/html)
2017-03-20 17:21 PDT, BJ Burg
no flags Details
WIP (2.55 KB, patch)
2017-03-24 11:15 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews116 for mac-elcapitan (1.92 MB, application/zip)
2017-03-24 16:44 PDT, Build Bot
no flags Details
Patch (3.17 KB, patch)
2017-04-03 17:03 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2017-03-20 17:21:35 PDT
Created attachment 304975 [details]
reduction

STEPS TO REPRODUCE:

1. Open the reduction
2. Scroll inside the div

EXPECTED:

During scrolling, overflow should be on the trailing side (left for RTL)

ACTUAL:

After scrolling a bit, the div shows the trailing content and the leading content is overflowed to the right side.
Comment 1 Radar WebKit Bug Importer 2017-03-20 17:21:55 PDT
<rdar://problem/31161158>
Comment 2 Simon Fraser (smfr) 2017-03-20 18:57:43 PDT
This sideways scroll is coming out of:

(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 6.1
  * frame #0: 0x0000000112d630d0 WebCore`WebCore::RenderLayer::scrollTo(this=0x00000001215684e0, position=0x00007fff58bc9c48) at RenderLayer.cpp:2369
    frame #1: 0x0000000112d65a23 WebCore`WebCore::RenderLayer::setScrollOffset(this=0x00000001215684e0, offset=0x00007fff58bc9d00) at RenderLayer.cpp:2796
    frame #2: 0x0000000113083b9f WebCore`WebCore::ScrollableArea::scrollPositionChanged(this=0x00000001215684e0, position=0x00007fff58bc9d58) at ScrollableArea.cpp:169
    frame #3: 0x0000000113083ff5 WebCore`WebCore::ScrollableArea::setScrollOffsetFromAnimation(this=0x00000001215684e0, offset=0x00007fff58bc9d98) at ScrollableArea.cpp:232
    frame #4: 0x0000000113088c06 WebCore`WebCore::ScrollAnimator::notifyPositionChanged(this=0x00000001215bcd80, delta=0x00007fff58bc9e10) at ScrollAnimator.cpp:195
    frame #5: 0x000000011308dca6 WebCore`WebCore::ScrollAnimatorMac::notifyPositionChanged(this=0x00000001215bcd80, delta=0x00007fff58bc9e10) at ScrollAnimatorMac.mm:770
    frame #6: 0x0000000113090053 WebCore`WebCore::ScrollAnimatorMac::immediateScrollBy(this=0x00000001215bcd80, delta=0x00007fff58bc9f48) at ScrollAnimatorMac.mm:1276
    frame #7: 0x000000011308ff86 WebCore`WebCore::ScrollAnimatorMac::immediateScrollByWithoutContentEdgeConstraints(this=0x00000001215bcd80, delta=0x00007fff58bc9f48) at ScrollAnimatorMac.mm:1263
    frame #8: 0x000000011309f6d1 WebCore`WebCore::ScrollController::snapRubberBandTimerFired(this=0x00000001215bcd98) at ScrollController.mm:340
    frame #9: 0x00000001130a2bf5 WebCore`WTF::RunLoop::Timer<WebCore::ScrollController>::fired(this=0x00000001215bcdd8) at RunLoop.h:139
    frame #10: 0x000000010dcad7a1 JavaScriptCore`WTF::RunLoop::TimerBase::timerFired((null)=0x00007ff5c8709f40, context=0x00000001215bcdd8) at RunLoopCF.cpp:84

from this logic:

        FloatPoint delta(roundToDevicePixelTowardZero(elasticDeltaForTimeDelta(m_startStretch.width(), -m_origVelocity.width(), (float)timeDelta)),
            roundToDevicePixelTowardZero(elasticDeltaForTimeDelta(m_startStretch.height(), -m_origVelocity.height(), (float)timeDelta)));

        if (fabs(delta.x()) >= 1 || fabs(delta.y()) >= 1) {
            m_client.immediateScrollByWithoutContentEdgeConstraints(FloatSize(delta.x(), delta.y()) - m_client.stretchAmount());

which I assume is not RTL-friendly (doesn't know about scrollOffset vs. scrollPosition).

Break in RenderLayer::scrollTo() when position.x() < 0 to catch this.
Comment 3 BJ Burg 2017-03-23 23:24:49 PDT
Patch written, will try to write a test tomorrow.
Comment 4 BJ Burg 2017-03-24 11:15:42 PDT
Created attachment 305301 [details]
WIP
Comment 5 Build Bot 2017-03-24 16:44:03 PDT
Comment on attachment 305301 [details]
WIP

Attachment 305301 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3406124

New failing tests:
media/video-main-content-autoplay.html
Comment 6 Build Bot 2017-03-24 16:44:06 PDT
Created attachment 305340 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 7 Simon Fraser (smfr) 2017-04-03 15:07:31 PDT
The real issue is that this logic is wrong in RTL:

    else if (scrollableContentsSize.width() && scrollOffset.x() > scrollableContentsSize.width() - visibleWidth())
        stretch.setWidth(scrollOffset.x() - (scrollableContentsSize.width() - visibleWidth()));

since scrollOffset.x() is not zero at rest.
Comment 8 Simon Fraser (smfr) 2017-04-03 17:03:05 PDT
Created attachment 306141 [details]
Patch
Comment 9 Build Bot 2017-04-03 17:05:40 PDT
Attachment 306141 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:13:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Simon Fraser (smfr) 2021-07-07 16:22:32 PDT
Seems fine now.