Bug 1666991. Remove PresShell::GetNearestScrollableFrame. r=kats

The current formulation is inconsistent. If you call it with ScrollableDirection::Either it goes into nsLayoutUtils::GetNearestScrollableFrame which will return the first scroll frame with a non-hidden overflow. If you call it with any other ScrollableDirection it calls nsLayoutUtils::GetNearestScrollableFrameForDirection which returns the first scrollframe it finds that has non-hidden overflow AND has at least one dev pixel of scroll range.

So remove that function and call nsLayoutUtils::GetNearestScrollableFrameForDirection directly. This is a slight change of behaviour but it seems desirable for all callers.

Differential Revision: https://phabricator.services.mozilla.com/D91236
This commit is contained in:
Timothy Nikkel 2020-09-24 13:44:54 +00:00
parent d2d21a52b6
commit 066a6ba348
5 changed files with 24 additions and 35 deletions

View File

@ -2866,18 +2866,6 @@ already_AddRefed<nsIContent> PresShell::GetSelectedContentForScrolling() const {
return selectedContent.forget();
}
nsIScrollableFrame* PresShell::GetNearestScrollableFrame(
nsIFrame* aFrame, ScrollableDirection aDirection) {
if (aDirection == ScrollableDirection::Either) {
return nsLayoutUtils::GetNearestScrollableFrame(aFrame);
}
return nsLayoutUtils::GetNearestScrollableFrameForDirection(
aFrame, aDirection == ScrollableDirection::Vertical
? nsLayoutUtils::eVertical
: nsLayoutUtils::eHorizontal);
}
nsIScrollableFrame* PresShell::GetScrollableFrameToScrollForContent(
nsIContent* aContent, ScrollableDirection aDirection) {
nsIScrollableFrame* scrollFrame = nullptr;
@ -2888,7 +2876,8 @@ nsIScrollableFrame* PresShell::GetScrollableFrameToScrollForContent(
if (scrollFrame) {
startFrame = scrollFrame->GetScrolledFrame();
}
scrollFrame = GetNearestScrollableFrame(startFrame, aDirection);
scrollFrame = nsLayoutUtils::GetNearestScrollableFrameForDirection(
startFrame, aDirection);
}
}
if (!scrollFrame) {
@ -2896,8 +2885,8 @@ nsIScrollableFrame* PresShell::GetScrollableFrameToScrollForContent(
if (!scrollFrame || !scrollFrame->GetScrolledFrame()) {
return nullptr;
}
scrollFrame =
GetNearestScrollableFrame(scrollFrame->GetScrolledFrame(), aDirection);
scrollFrame = nsLayoutUtils::GetNearestScrollableFrameForDirection(
scrollFrame->GetScrolledFrame(), aDirection);
}
return scrollFrame;
}

View File

@ -87,6 +87,8 @@ class ZoomConstraintsClient;
struct nsCallbackEventRequest;
enum class ScrollableDirection;
namespace mozilla {
class AccessibleCaretEventHub;
class EventStates;
@ -429,15 +431,6 @@ class PresShell final : public nsStubDocumentObserver,
nsIScrollableFrame* GetScrollableFrameToScroll(
ScrollableDirection aDirection);
/**
* Gets nearest ancestor scrollable frame from aFrame. The frame is
* scrollable with overflow:scroll or overflow:auto in some direction when
* aDirection is eEither. Otherwise, this returns a nearest frame that is
* scrollable in the specified direction.
*/
nsIScrollableFrame* GetNearestScrollableFrame(nsIFrame* aFrame,
ScrollableDirection aDirection);
/**
* Returns the page sequence frame associated with the frame hierarchy.
* Returns nullptr if not a paginated view.

View File

@ -149,8 +149,6 @@ enum class ScrollFlags {
MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(ScrollFlags)
enum class ScrollableDirection { Horizontal, Vertical, Either };
// See comment at declaration of RenderDocument() for the detail.
enum class RenderDocumentFlags {
None = 0,

View File

@ -2087,7 +2087,7 @@ bool nsLayoutUtils::IsFixedPosFrameInDisplayPort(const nsIFrame* aFrame) {
// static
nsIScrollableFrame* nsLayoutUtils::GetNearestScrollableFrameForDirection(
nsIFrame* aFrame, Direction aDirection) {
nsIFrame* aFrame, ScrollableDirection aDirection) {
NS_ASSERTION(
aFrame, "GetNearestScrollableFrameForDirection expects a non-null frame");
for (nsIFrame* f = aFrame; f; f = nsLayoutUtils::GetCrossDocParentFrame(f)) {
@ -2095,12 +2095,20 @@ nsIScrollableFrame* nsLayoutUtils::GetNearestScrollableFrameForDirection(
if (scrollableFrame) {
ScrollStyles ss = scrollableFrame->GetScrollStyles();
uint32_t directions = scrollableFrame->GetAvailableScrollingDirections();
if (aDirection == eVertical
? (ss.mVertical != StyleOverflow::Hidden &&
(directions & nsIScrollableFrame::VERTICAL))
: (ss.mHorizontal != StyleOverflow::Hidden &&
(directions & nsIScrollableFrame::HORIZONTAL)))
return scrollableFrame;
if (aDirection == ScrollableDirection::Vertical ||
aDirection == ScrollableDirection::Either) {
if (ss.mVertical != StyleOverflow::Hidden &&
(directions & nsIScrollableFrame::VERTICAL)) {
return scrollableFrame;
}
}
if (aDirection == ScrollableDirection::Horizontal ||
aDirection == ScrollableDirection::Either) {
if (ss.mHorizontal != StyleOverflow::Hidden &&
(directions & nsIScrollableFrame::HORIZONTAL)) {
return scrollableFrame;
}
}
}
}
return nullptr;

View File

@ -141,6 +141,8 @@ enum class DrawStringFlags {
};
MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(DrawStringFlags)
enum class ScrollableDirection { Horizontal, Vertical, Either };
/**
* nsLayoutUtils is a namespace class used for various helper
* functions that are useful in multiple places in layout. The goal
@ -673,9 +675,8 @@ class nsLayoutUtils {
* @param aDirection Whether it's for horizontal or vertical scrolling.
* @return the nearest scrollable frame or nullptr if not found
*/
enum Direction { eHorizontal, eVertical };
static nsIScrollableFrame* GetNearestScrollableFrameForDirection(
nsIFrame* aFrame, Direction aDirection);
nsIFrame* aFrame, ScrollableDirection aDirection);
enum {
/**