Transcript scroll position lost when opening / closing sidebar in Safari

[robertknight]

Opening and closing the sidebar in the video player can cause the displayed location of the transcript to shift significantly, if the browser does not support scroll anchoring. In my testing, Chrome's scroll anchoring seems to work well. Firefox's scroll anchoring did not always work the first time that the sidebar was opened, but did work after that. The main issue is in Safari which doesn't support scroll anchoring.

Sidebar closed, note visible timestamps:

After opening the sidebar, the visible range of timestamps changes significantly:

Tested with https://qa-via.hypothes.is/https://www.youtube.com/watch?v=MrORaCfcKxQ but should apply to any video.

[robertknight]

The issue of the current scroll position being lost mainly occurs when opening and closing the sidebar, but the same thing can occur when resizing the window and a transition between the different layout breakpoints occurs. Therefore we probably want to respond not only to the sidebar opening/closing, but probably any event which causes the transcript container to be resized.

[robertknight]

Safari issues: https://bugs.webkit.org/show_bug.cgi?id=171099, https://bugs.webkit.org/show_bug.cgi?id=243702. There seems to be consensus that WebKit should implement this, but we'll need a workaround in the interim.

[acelaya]

There seems to be consensus that WebKit should implement this, but we'll need a workaround in the interim.

@robertknight do you mean we should work around this specifically in the video app, or in the client in general?

I can reproduce this in any app by setting scroll-anchor: none, but I guess it's less critical (for us) that it happens in other places.

[robertknight]

@robertknight do you mean we should work around this specifically in the video app, or in the client in general?

Specifically in the video app. We have a general solution that works in all browsers for the client's "automatic" side-by-side mode. Search for preserveScrollPosition in the HTMLIntegration class. This general solution doesn't work for the video player because the content is in a separate scrollable container (the transcript) rather than the video player app. We could perhaps make the client's general solution support this, but since we plan to have the video player app "take control" of side-by-side mode itself, I think it makes sense to implement the solution here instead.

[acelaya]

I'm temporarily pausing work on this, as one of the events we need to listen to is the custom hypothesis:layoutchange.

Since this is also going to be listened to as part of https://github.com/hypothesis/client/issues/5571, we may conflict or duplicate logic.

I'll wait for that to be solved, and then decide if it makes sense to re-use whatever logic is used there to listen for that event, or it's better to add a different listener for the same event.

[acelaya]

I'm temporarily pausing work on this, as one of the events we need to listen to is the custom hypothesis:layoutchange.

Since this is also going to be listened to as part of hypothesis/client#5571, we may conflict or duplicate logic.

I'll wait for that to be solved, and then decide if it makes sense to re-use whatever logic is used there to listen for that event, or it's better to add a different listener for the same event.

Discussed with @lyzadanger how the hypothesis:layoutchange event is handled there, and we both agree it probably makes more sense to listen to the event separately for this purpose.

I will resume work on this task.

[acelaya]

I'm struggling a bit to get this fully working, but I'm going to explain what I have tried so far:

Using preserveScrollPosition from client

My first attempt involved using preserveScrollPosition from client (or a variation of it), for which I copy-pasted that function and its dependencies, just to validate if it was a viable option.

The first thing I realized is that this function assumes we are in control of the logic that can potentially make the scrollable element to resize, which is not the case here.

In this case, we need to react to events and re-compute scroll then, which means we have to track the previous value so that when the event happens we can compare the old and new values and calculate the scroll delta.

In order to do that, I added a scroll event listener.

Note It's not entirely related with the approach, but at this point I'm experimenting with using a ResizeObserver instead of reacting to the hypothesis:layoutchange event, as it covers more cases. However, this ResizeObserver + the scroll listener mentioned above can become resource-greedy, so we would need to keep an eye on this.

Once I got over that limitation, I realized the top value from the Range object returned by getScrollAnchor was always a relatively small value. My conclusion is that it's the distance from the top of the viewport, not the top of the scroll, which is enough to re-compute top scroll in the case of side-by-side in the client, but here, the scroll can significantly vary between layout changes.

At this point I decided to try something else.

Compute scroll using a "rule of three"

This approach makes the next assumptions:

If the scrollable component is 50k px high and the scroll top is 30k px, when the component is resized, changing the heght to, let's say 60k px, we can compute the new scroll top by doing 30k / 50k * 60k, or oldScrollTop / oldHeight * newHeight.

We can track the value of oldScrollTop / oldHeight using a scroll event listener, and do the trackedValue * newHeight using the ResizeObserver.

This approach kind-of works, but since every segment is not a "fixed box", but a piece of text, there might be cases in which some of them keep their size after resizing (they don't span more lines), making the rule above less precise.

In my testing so far, this leaves you very "close" to the segment you were before resizing, but not exactly in the same position.

[robertknight]

In this case, we need to react to events and re-compute scroll then, which means we have to track the previous value so that when the event happens we can compare the old and new values and calculate the scroll delta. In order to do that, I added a scroll event listener.

I think this is going in the right direction, but the solution here can be completely specialized to the Transcript component and not necessarily reuse any of preserveScrollPosition implementation if the API isn't a good fit.

I am thinking it would look something like this:

1. When the transcript is scrolled, or after a render, find the transcript segment that is visible at the top of the container and save it. Also save the offset of the top of that segment from the top of the container.
2. Detect when the transcript container is resized, and compute the new scrollTop that would position the saved segment in the same place within the scrollable container.
3. Adjust the container's scrollTop to the computed position
4. Turn off scroll anchoring for the container, in browsers that support this, so we don't get the browser's native behavior fighting with our own.

[robertknight]

This approach kind-of works, but since every segment is not a "fixed box", but a piece of text, there might be cases in which some of them keep their size after resizing (they don't span more lines), making the rule above less precise.

I do think we need to use the actual height of segments, and probably won't be able to get away with measuring one and assuming a fixed height. As you've noted, they can vary in length. For a long transcript, mismatches between the estimated and actual offsets of segments will accumulate and could be significantly off towards the end.

[acelaya]

1. When the transcript is scrolled, or after a render, find the transcript segment that is visible at the top of the container and save it. Also save the offset of the top of that segment from the top of the container.
2. Detect when the transcript container is resized, and compute the new scrollTop that would position the saved segment in the same place within the scrollable container.
3. Adjust the container's scrollTop to the computed position

I think I tried this at some point, but could not find a way to determine the "scroll top distance" of that element, just the "viewport top distance", but I'll give it another try, as this would be the best option.

4. Turn off scroll anchoring for the container, in browsers that support this, so we don't get the browser's native behavior fighting with our own.

Yep, I think I have this part covered using CSS.supports('overflow-anchor: auto'), but I'm delaying this to the end for now.

[robertknight]

I think I tried this at some point, but could not find a way to determine the "scroll top distance" of that element, just the "viewport top distance", but I'll give it another try, as this would be the best option.

A few approaches you can try:

• Call getBoundingClientRect on the scrollable container and descendant, and compare their top offsets. The descendant's offset will be less than the container's offset if it's top is off-screen.
• If you have scrollable container that is positioned, and a descendant whose nearest positioned ancestor (aka. its offsetParent) is the scroll container, you can compare the descendant's offsetTop to the container's scrollTop.

[acelaya]

@robertknight I have created a draft https://github.com/hypothesis/via/pull/1067, following your suggestion here https://github.com/hypothesis/via/issues/1021#issuecomment-1630501371

I think it works pretty well in general, but I guess there are possible adjustments to be made.