New issue
Advanced search Search tips

Issue 922456 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

WebViewImpl::EnableDeviceEmulation() doesn't always properly change scrollbar style

Project Member Reported by mstensho@chromium.org, Jan 16 (6 days ago)

Issue description

There's a unit test ScrollbarAppearanceTest.NativeScrollbarChangeToMobileByEmulator that calls WebViewImpl::EnableDeviceEmulation(). This should change scrollbars to overlay scrollbars. The test passes with the legacy layout engine. It fails in LayoutNG.

The reason that it passes in legacy layout, though, is that legacy layout isn't as good at skipping subtrees that don't need layout as LayoutNG. In general it goes one level too deep. See https://bugs.chromium.org/p/chromium/issues/detail?id=667370#c3

Modifying the HTML in the unit test a little makes it fail in legacy too.

    <!DOCTYPE html>
    <style type='text/css'>
    body {
      height: 10000px;
      margin: 0;
    }
    #d1 {
      height: 200px;
      width: 200px;
      overflow: auto;
    }
    #d2 {
      height: 2000px;
    }
    </style>
    <div id='d1'>
      <div id='d2'/>
    </div>

If we wrap #d1 inside a DIV with fixed width, it starts to fail in legacy layout, too.

It seems that there's no mechanism to ensure that all scrollbars in the entire document are replaced when needed, and that we essentially just rely on luck.
 

Comment 1 by mstensho@chromium.org, Jan 16 (6 days ago)

Cc: e...@chromium.org szager@chromium.org
Components: Platform>DevTools>Mobile Blink>Layout Blink>Scroll
Status: Available (was: Untriaged)
Attaching test (interactive; requires entering and leaving devtools). Fails in both LayoutNG and legacy. Removing the width:300px declaration on the outer div should make it pass in legacy (because of the aforementioned subtree-skipping deficiency in the legacy engine), but not in LayoutNG.
tc.html
872 bytes View Download

Sign in to add a comment