Here's the relevant rule in Document::scrollingElement:

if (inQuirksMode()) {
      updateStyleAndLayoutTree();
      HTMLBodyElement* body = firstBodyElement();
      if (body && body->layoutObject() &&
          body->layoutObject()->hasOverflowClip())
        return nullptr;

      return body;
}

While the spec says: 

"If the HTML body element exists, and it is not potentially scrollable, return the HTML body element and abort these steps." which translates to:

if (body && !(body->layoubObject() && body->layoutObject()->hasOverflowClip()))
  return body

The body element is only "potentially scrollable" if the root element’s used value of the overflow-x or overflow-y properties is not visible. I think we need to look at the style on documentElement here.

Chao, please take a look when you have some time.

Q2 2017 Update: Issue is still relevant.

This has to do with how we check "potentially scrollable". From the spec (https://drafts.csswg.org/cssom-view/#potentially-scrollable):

An element is potentially scrollable if all of the following conditions are true:

- The element has an associated CSS layout box.
- The element is not the HTML body element, or it is and the root element’s used value of the overflow-x or overflow-y properties is not visible.
- The element’s used value of the overflow-x or overflow-y properties is not visible.

So for the body element we need to look at the hasOverflowClip of the root element (i.e. documentElement). In the test, we fail because documentElement.style.overflow becomes visible but we're still returning nullptr. We should be failing the `if`-returning nullptr and falling into the body return;

So there are two bugs

1) https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Document.cpp?dr&l=2910 is wrong in that it doesn't check if the document is a HTML document...

2) There seems to be a recalculation issue for propagating the HasOverflowClip bit...

If I add this (dirtying the style on the document) the test passes correctly.
    quirksDoc.body.style = quirksDoc.body.style;

    assert_equals(quirksDoc.scrollingElement, quirksDoc.body);

eae@, chrishtr@ any idea why HasOverflowClip isn't updated on the body? when a parent's overflow style changes?

I think the sequence is like this:
    quirksDoc.documentElement.style.overflow = "scroll";
    quirksDoc.body.style.overflow = "scroll";
    quirksDoc.documentElement.style.overflow = "visible";
body doesn't have overflow clip here..
but if I add:
    quirksDoc.body.style = quirksDoc.body.style;
Then it does

I don't know, sorry. Adding trchen who might know.

Sounds related to issues  541529 ,  731022  and  590818 .

Perhaps this case just isn't handled by the Document::InheritHtmlAndBodyElementStyles logic that tries to correctly propagate the style changes given the crazy rules?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fc202b2b1174f5732f7b9e63a6c46fd01ac2be02

commit fc202b2b1174f5732f7b9e63a6c46fd01ac2be02
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Fri Jun 09 13:45:06 2017

Change FirstBodyElement to check the document is a HTML Document.

The CSS spec specifies that the document must be a HTML Document.

Layout test asserts this:
http://w3c-test.org/cssom-view/scrollingElement.html

There is one other user of FirstBodyElement since it was first
added. The background style propagation from the HTML body
element to the HTML element. It appears before it was rewritten
it was checking that the document was a HTML document as well.
So I think this is safe to make the API match the spec.

BUG= 157855 , 665927 

Change-Id: Iafbe7c25011cdde2c89b3d839dcfb08e5bc12c3e
Reviewed-on: https://chromium-review.googlesource.com/527462
Reviewed-by: Rick Byers <rbyers@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478262}
[modify] https://crrev.com/fc202b2b1174f5732f7b9e63a6c46fd01ac2be02/third_party/WebKit/Source/core/dom/Document.cpp

+1 to #11, I'd guess that the style code doesn't know (at least in this case) to dirty style that's shared between <html> and <body> on the other element.

rune@ are you looking at the other style propagation issues for the documentElement?

Currently shipping the scrollTopInterop is blocked on this wpt test failure. Just wondering if I should continue looking at it or you have a fix in the works.

As per comment #3 dirtying the style in javascript makes this test work. I'm convinced something is odd with the style recalc system.

Assinging to rune@ to investigate/delegate.

Did a quick test and my WIP change doesn't currently fix the w3c test.

Tested this issue on Ubuntu 14.04, Windows-10 and Mac OS 10.12.5 using chrome latest dev #61.0.3129.0 by following steps mentioned in the original comment. Observed assert_equals still displays fails.

rune@ are you able to debug this and understand why adjusting the style makes this work? This is a relatively stared feature for interop that we'd like to launch so I'd appreciate it if you could devote some time to it.

If not then I'll have to roll back our intervention for mousewheel which has already landed in ToT because launching the scrollTopLeft interop wouldn't happen shortly.

Maybe this is actually orthogonal from shipping scrollTopLeftInterop?  There's a pre-existing problem here regardless, right?

I'll have a look

There was a spec bug for "potentially scrollable" which I've discussed with simonp@ now, and a spec PR in progress, but that's not directly relevant to the bug, I think.

Based on comment #7, I made this simple case which correctly logs the body element (enabled experimental features):

<!-- quirks -->
<body>
<script>
  document.documentElement.style.overflow = "scroll";
  document.body.style.overflow = "scroll";
  document.offsetTop;
  document.documentElement.style.overflow = "visible";
  console.log(document.scrollingElement);
</script>

so, there must be a different twist in the failing test.

I know what the problem is now, but not sure how to fix this is a pretty way.

UpdateFromStyle() on the layout object sets the HasOverflowClip() flag as part of setting ComputedStyle. That means updating this flag only happens when recalculating style. Since this flag on body depends on the computed style of html, we fail to update the flag on body when html is recalculated but body does not need to be recalculated. Forcing a body recalc does not work because if the style doesn't change, UpdateFromStyle is not called. Checking if ViewportDefiningElement() changes in InheritHtmlAndBodyElementStyles() could work, but the UpdateFromStyle() depends on the computed style being set, while the former method does not do that.

I'll figure something out.

https://chromium-review.googlesource.com/c/540596/

rune@ you are awesome! Many thanks for working on this!

thanks. np.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1c6703ce6739d5e8725844bbed2c2e938b54e863

commit 1c6703ce6739d5e8725844bbed2c2e938b54e863
Author: Steve Kobes <skobes@chromium.org>
Date: Wed Jun 21 20:21:54 2017

Revert "Update HasOverflowClip for body after style recalc."

This reverts commit 826817e52d02 (http://crrev.com/480824).

Reason for revert: Identified as cause of crash regression in
http://crbug.com/735371.

TBR=rbyers@chromium.org,rune@opera.com,dtapuska@chromium.org
Bug:  665927 , 735371

Change-Id: I1721851eb654e14fb3a697acce1f5c6f0cc2fdcb
Reviewed-on: https://chromium-review.googlesource.com/543540
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481289}
[add] https://crrev.com/1c6703ce6739d5e8725844bbed2c2e938b54e863/third_party/WebKit/LayoutTests/external/wpt/cssom-view/HTMLBody-ScrollArea_quirksmode-expected.txt
[modify] https://crrev.com/1c6703ce6739d5e8725844bbed2c2e938b54e863/third_party/WebKit/LayoutTests/external/wpt/cssom-view/HTMLBody-ScrollArea_quirksmode.html
[add] https://crrev.com/1c6703ce6739d5e8725844bbed2c2e938b54e863/third_party/WebKit/LayoutTests/external/wpt/cssom-view/scrollingElement-expected.txt
[modify] https://crrev.com/1c6703ce6739d5e8725844bbed2c2e938b54e863/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/1c6703ce6739d5e8725844bbed2c2e938b54e863/third_party/WebKit/Source/core/dom/Document.h
[modify] https://crrev.com/1c6703ce6739d5e8725844bbed2c2e938b54e863/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
[modify] https://crrev.com/1c6703ce6739d5e8725844bbed2c2e938b54e863/third_party/WebKit/Source/core/layout/LayoutBlock.h

Reverted due to the crash exposed in attached test case.

Deleted: overflow.html 207 bytes

overflow.html 207 bytes View Download

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1edc94d19d773c1459f73cf9ed52b70cf209288e

commit 1edc94d19d773c1459f73cf9ed52b70cf209288e
Author: Rune Lillesveen <rune@opera.com>
Date: Thu Jun 22 21:59:55 2017

Update style for body when viewport defining element changes.

Whether the body box has scrolling overflow or not depends on the
computed overflow of the html element.

The HasOverflowClip flag, along with creating and removing a paint
layer on LayoutBlock is updated as part of SetStyle. However, if the
html element is recalculated, its overflow changing causing the
viewport defining element to change, but body did not need a recalc,
the overflow clip flag and paint layer is not updated for body.

This CL forces a SetStyle on the body LayoutObject to trigger the
necessary update after recalc when when the viewport defining element
changes.

This fixes scrollingElement.html in wpt/cssom-view.

Bug:  665927 
Change-Id: I146c3e976edef28074bde6531fe4c6ec65ecb090
Reviewed-on: https://chromium-review.googlesource.com/544958
Commit-Queue: Rune Lillesveen <rune@opera.com>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Rick Byers <rbyers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481682}
[delete] https://crrev.com/15399f887a0e8498253dc4496446eee83158e96a/third_party/WebKit/LayoutTests/external/wpt/cssom-view/HTMLBody-ScrollArea_quirksmode-expected.txt
[modify] https://crrev.com/1edc94d19d773c1459f73cf9ed52b70cf209288e/third_party/WebKit/LayoutTests/external/wpt/cssom-view/HTMLBody-ScrollArea_quirksmode.html
[delete] https://crrev.com/15399f887a0e8498253dc4496446eee83158e96a/third_party/WebKit/LayoutTests/external/wpt/cssom-view/scrollingElement-expected.txt
[add] https://crrev.com/1edc94d19d773c1459f73cf9ed52b70cf209288e/third_party/WebKit/LayoutTests/external/wpt/cssom-view/scrollingElement-quirks-dynamic-001-ref.html
[add] https://crrev.com/1edc94d19d773c1459f73cf9ed52b70cf209288e/third_party/WebKit/LayoutTests/external/wpt/cssom-view/scrollingElement-quirks-dynamic-001.html
[add] https://crrev.com/1edc94d19d773c1459f73cf9ed52b70cf209288e/third_party/WebKit/LayoutTests/external/wpt/cssom-view/scrollingElement-quirks-dynamic-002-ref.html
[add] https://crrev.com/1edc94d19d773c1459f73cf9ed52b70cf209288e/third_party/WebKit/LayoutTests/external/wpt/cssom-view/scrollingElement-quirks-dynamic-002.html
[modify] https://crrev.com/1edc94d19d773c1459f73cf9ed52b70cf209288e/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/1edc94d19d773c1459f73cf9ed52b70cf209288e/third_party/WebKit/Source/core/dom/Document.h