[cssom-view-1] Element.scrollIntoView() does not scroll to a position:fixed element in an iframe

[Bammuri]

While contributing to Chromium, I discovered an interesting issue. Element.scrollIntoView() does not scroll to a position:fixed element in an iframe

There is something to be discussed before addressing the issue.

How should we treat elements that are offscreen and can't be scrolled on screen? e.g. https://jsbin.com/qehegek

Bokan@ said, if, after scrolling the immediate-most parent the element isn't visible in the parent, should we keep recursing up the scroll chain? I'm not sure what the right answer is (my hunch would be "no" but could easily be convinced)

I agree, and I think we should handle exceptions in these cases. If there are other cases, please tell me:)

[bokand]

@emilio @smfr since I came across this recently again while fixing a bug in a related area.

I've written a test case to help show the different behaviors: https://mparch-bokan.glitch.me/bug/scrollIntoView-fixed.html

Each orange box is position: fixed with a child target that gets scrollIntoView({block:'start', inline:'start'}) called on it:

Box A: Is unscrollable Box B: Is scrollable Box C: Is unscrollable and inside an iframe Box D: Is scrollable and inside an iframe

Bubbling Behavior

Box A: No scroll - all engines agree Box B: Container scrolls, no window scroll - all engines agree

Box C:

• Safari: Scrolls outer window so that target is in top left corner
• Firefox: No scroll
• Chrome: No scroll

Box D:

• Safari: Scrolls container, scrolls outer window
• Firefox: Scrolls container, scrolls outer window
• Chrome: Scrolls container

IMO Chrome's behavior is clearly a bug in the C/D cases. I also think the Firefox behavior in C seems like a bug; the fact that the container is scrollable shouldn't make the different of whether to bubble the scrollIntoView up to the outer window. I'd propose adopting Safari's behavior in the spec, WDYT?

Clipping Behavior

If you tick the checkbox it'll shift the position: fixed elements so that the target, after being scrolled into view in the container, is outside of the frame.

Box A: No scroll - all engines agree Box B: Container scrolls - all engines agree

Box C:

• Safari - Scrolls outer window to target location (i.e. where the target would be if it weren't clipped)
• Firefox - No Scroll
• Chrome - No Scroll

Box D:

• Safari - Scrolls container then scrolls outer window to target location
• Firefox - Scrolls container then scrolls outer window to target location
• Chrome - Scrolls container

When I fix the bug for the bubbling behavior Chrome has the same behavior as Safari but it seems unintuitive to me to scroll something into view that isn't visible.

I wonder though if changing that is web-compatible. In Chrome we intersect the target's rect to the scroll port rect each time we bubble up but we have an edge-case that sets an empty rect dimension to 1px that IIRC we had to add for web compat.

I'll try to put together a more exhaustive test set for how scrollIntoView behaves w.r.t. overflow clips but any thoughts you may have here would be helpful.

[emilio]

Yeah, I think the Firefox behavior is because this code doesn't look across document boundaries, and that looks like a bug.

My only concern with scrolling the outer window is whether <iframe>s (in particular cross-origin ones) should really do that. Is there any information they can infer information that they couldn't / shouldn't (e.g., via IntersectionObserver)?

But we right now are scrolling stuff into view on them when they have a scrollable ancestor, if I'm reading the code correctly, so it seems it wouldn't really make stuff worse.

So assuming the cross-origin info leaks are not a worry, I think safari's behavior makes sense in the unclipped case.

For the clipping case: I don't think we do any intersection shenanigans in Gecko. It's weird to special-case clipping but not other ways of occlusion / hiding content, I think?

If we don't intersect, then we get the Safari behavior, right? I think that may be the simplest one to spec and explain to developers (and I don't think I've seen any compat issue with this).

[emilio]

Yeah, can confirm that with a simple fix like this:

diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp
index bb5313563e624..a2e0f47232050 100644
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -3669,8 +3669,7 @@ void PresShell::DoScrollContentIntoView() {

   // Make sure we skip 'frame' ... if it's scrollable, we should use its
   // scrollable ancestor as the container.
-  nsIFrame* container = nsLayoutUtils::GetClosestFrameOfType(
-      frame->GetParent(), LayoutFrameType::Scroll);
+  nsIFrame* container = frame->GetParent();
   if (!container) {
     // nothing can be scrolled
     return;

I get the expected behavior for box C.

[emilio]

The real patch happened to be a little bit more subtle, but I attached it to https://bugzilla.mozilla.org/show_bug.cgi?id=1763743. If we agree on that behavior I'm happy to land that.

[bokand]

My only concern with scrolling the outer window is whether