1215974 - in nsLayoutUtils::GetNearestScrollableFrame don't skip the root scroll frame if we are asked to always match the root scroll frame even if it doesn't WantAsyncScroll() (and we are also asked to match only async scrollable frames)

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Timothy Nikkel (:tnikkel)]

APZ needs a scrollable frame otherwise events might be lost. So if we are asked to always match root and only match async scrollable we need to return the root scroll frame even if it isn't "WantAsyncScroll()" because otherwise we will return null since we have nothing else to return.

APZ is the only user of these two flags.

Comment 1

[Timothy Nikkel (:tnikkel)]

Seems like this doesn't cause problems because PrepareForSetTargetAPZCNotification in APZCCallbackHelper (the only user) will fall back to the root element, and should almost always already have a displayport before PrepareForSetTargetAPZCNotification is called.

Comment 2

[Timothy Nikkel (:tnikkel)]

Attachment: patch

Comment 3

[Botond Ballo [:botond]]

Comment on attachment 8675447 [details] [diff] [review]
patch

Review of attachment 8675447 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsLayoutUtils.cpp
@@ +1909,5 @@
> +        if (!(aFlags & SCROLLABLE_ONLY_ASYNC_SCROLLABLE) ||
> +            scrollableFrame->WantAsyncScroll()) {
> +          return scrollableFrame;
> +        }
> +      }

I would find the following more readable:

if (aFlags & SCROLLABLE_ONLY_ASYNC_SCROLLABLE) {
  if (scrollableFrame->WantAsyncScroll()) {
    return scrollableFrame;
  }
} else {
  ScrollbarStyles ss = scrollableFrame->GetScrollbarStyles();
  if ((aFlags & SCROLLABLE_INCLUDE_HIDDEN) ||
      ss.mVertical != NS_STYLE_OVERFLOW_HIDDEN ||
      ss.mHorizontal != NS_STYLE_OVERFLOW_HIDDEN) {
    return scrollableFrame;
  }
}

Comment 4

[Timothy Nikkel (:tnikkel)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8507e88cd18b

Comment 5

[Timothy Nikkel (:tnikkel)]

(In reply to Botond Ballo [:botond] from comment #3)
> I would find the following more readable:

I agree, I changed it.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/394230165a48

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/394230165a48