Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storeScrollPos / resetScrollPos conflict with Chrome scroll anchoring #933

Closed
matej-svejda opened this issue May 27, 2019 · 8 comments

Comments

Projects
None yet
2 participants
@matej-svejda
Copy link

commented May 27, 2019

Issue details

ProseMirror tries to persist what part of a document is visible, even if above the visible viewport DOM nodes are added/removed. This functionallity is implemented in prosemirror-view in the storeScrollPos/resetScrollPos functions, also if the scrolling happens inside a container element other than the body.
However, Chrome does this automatically by itself since version 56 (see https://www.chromestatus.com/feature/5700102471548928). This leads to the content scrolling with the current storeScrollPos/resetScrollPos implementation.

Workaround is to set overflow-anchor: none; on the editor container. However, I think it would be better to take advantage of this functionallity, since the preserve scroll implemention of ProseMirror seems to have minor calculation errors sometimes.

Steps to reproduce

To reproduce in Chrome, simply put the editor into a fixed-height container with overflow auto, scroll somewhere to the middle of the document and instert something at the beginning:

https://glitch.com/edit/#!/join/89fb4025-d89a-41b8-9c12-20d3f420f4bd

ProseMirror version

Prosemirror view version 1.9.6

Affected platforms

  • Chrome >= 56
@marijnh

This comment has been minimized.

Copy link
Member

commented May 27, 2019

Hm. I expect that the Chrome behavior won't always work (when the node it uses as a reference is redrawn), but I guess it should usually be fine. There's overflow-anchor: none, which we could use to suppress the behavior, but the issue there is that the scrollable container might be outside of ProseMirror, so we can't control its style.

Trying to feature-detect this (with something like element.style.overflowAnchor != null) and disabling ProseMirror's behavior when it's present might be acceptable. But that style is defined in Firefox as well—I haven't check yet whether Firefox implements this behavior.

I'll look into this more closely later.

marijnh added a commit to ProseMirror/prosemirror-view that referenced this issue May 28, 2019

Don't stabilize the scroll position when the browser supports scroll …
…anchoring

FIX: ProseMirror will no longer try to stabilize the scroll position
during updates on browsers that support [scroll
anchoring](https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-anchor),
since it'd inadvertently cancel the browser's behavior.

Issue ProseMirror/prosemirror#933
@marijnh

This comment has been minimized.

Copy link
Member

commented May 28, 2019

Attached patch turns off this behavior on browsers that do overflow anchoring.

@marijnh marijnh closed this May 28, 2019

@matej-svejda

This comment has been minimized.

Copy link
Author

commented May 28, 2019

For me this doesn't work in Firefox on Linux and Mac (didnt test on windows): the content still scrolls down if there are edits made above the visible area. Seems like Firefox and Chrome implment overflow-anchor: auto differently.

@marijnh

This comment has been minimized.

Copy link
Member

commented May 28, 2019

Weird, I tested Firefox Linux. What kind of edit are you making?

@matej-svejda

This comment has been minimized.

Copy link
Author

commented May 28, 2019

Just basic insert/remove operations in a collaborative editor.

@marijnh

This comment has been minimized.

Copy link
Member

commented May 29, 2019

Odd. With Firefox 67 Linux, both with a scrollable element and with the document being the scrollable container, scrolling halfway down the document and then repeatedly deleting the first node in the document shows a stable scroll position.

@matej-svejda

This comment has been minimized.

Copy link
Author

commented May 30, 2019

I only tested it with my own project, not with the minimal example I created for this issue. I'll try to reproduce it there and then get back to you (I'll probably get around to it on friday).

@matej-svejda

This comment has been minimized.

Copy link
Author

commented Jun 2, 2019

Sorry I didnt get around to it when I said I would. But I now managed to find the problem.

TLDR: It was my own code.

More detailed: Im using https://www.npmjs.com/package/resize-detector to track size changes and was doing so on one of the editor's parent elments. In Firefox, there is no native ResizeObserver, so it falls back to adding a hidden element for tracking resize events, which caused the overflow-anchor not to work... Anyways: sorry for the confusion and thanks for the quick fix :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.