[cssom-view] scrollIntoView container spec unclear

[zcorpan]

#11673 added container dictionary member to scrollIntoView(options). From #9452 it seems there was discussion about supporting specifying a particular element directly, but then decided to only support the strings "nearest" and "all". However, the spec seems to have artifacts of both.

https://drafts.csswg.org/cssom-view/#dictdef-scrollintoviewoptions

If the container dictionary member of options,

If the container is what?

set container to this element.

What does "this element" mean?

https://drafts.csswg.org/cssom-view/#scroll-a-target-into-view

and an optional containing Element to stop scrolling after reaching container

If container is not null and scrolling box is a shadow-including inclusive ancestor of container, abort the rest of these steps.

What if the nearest scrolling box is a viewport?

cc @flackr @tabatkins

[flackr]

#11673 added container dictionary member to scrollIntoView(options). From #9452 it seems there was discussion about supporting specifying a particular element directly, but then decided to only support the strings "nearest" and "all". However, the spec seems to have artifacts of both.

The container attribute is also used by the overflow-5 spec where it is not set to the element: https://drafts.csswg.org/css-overflow-5/#scroll-marker-activation, although given scroll markers associate with the nearest scroll container, perhaps it is equivalent to "nearest" and could be removed. I had still been thinking that it may be useful to have the algorithm allow arbitrary containing elements even if the JS api only allowed using the nearest container for now.

https://drafts.csswg.org/cssom-view/#dictdef-scrollintoviewoptions

If the container dictionary member of options,

If the container is what?

Oops, this was clearly missed, if container is "nearest".

set container to this element.

What does "this element" mean?

It's the same "the element" that is referenced by the rest of the algorithm, i.e. the element on which scrollIntoView was called. I've switched it to "the element" for consistency to avoid confusion.

https://drafts.csswg.org/cssom-view/#scroll-a-target-into-view

and an optional containing Element to stop scrolling after reaching container

If container is not null and scrolling box is a shadow-including inclusive ancestor of container, abort the rest of these steps.

What if the nearest scrolling box is a viewport?

The intent is that we would stop at the viewport if its document contains the container (i.e. "is a shadow-including inclusive ancestor of"). Is the issue that the viewport is not a scrolling box (https://drafts.csswg.org/cssom-view/#scrolling-box seems to suggest it is), or is it that the viewport is not an ancestor of the element and this case needs to be specifically spelled out?

I've uploaded a PR to address these in #12354, take a look and feel free to suggest any needed improvements.

[flackr]

although given scroll markers associate with the nearest scroll container

Actually, element scroll markers established with scroll-target-group have a scroll container that may not be the nearest scroll container.