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

doc: weird scrolling/anchor behavior in Chrome and Firefox #40099

Closed
mscdex opened this issue Sep 13, 2021 · 34 comments · May be fixed by #41869
Closed

doc: weird scrolling/anchor behavior in Chrome and Firefox #40099

mscdex opened this issue Sep 13, 2021 · 34 comments · May be fixed by #41869
Labels
confirmed-bug Issues with confirmed bugs. doc Issues and PRs related to the documentations.

Comments

@mscdex
Copy link
Contributor

mscdex commented Sep 13, 2021

📗 API Reference Docs Problem

Location

Affected URL(s):

Description

At least on Linux and with Chrome, if you load a very long page, like the CommonJS modules API page, and you either press the End key or otherwise scroll to the bottom and then attempt to scroll up or hold down the Page Up key, the scroll bar makes a lot of strange jumps (backwards and forwards). Even starting at the top of the page and scrolling down causes the scroll bar to do some large, sudden, and unexpected jumps. However, using the Home and End keys seemingly work just fine.

My guess is there is either some javascript on the page listening for scrolling events in general or on particular elements (like code blocks) and that is somehow causing the weird issues.

@mscdex mscdex added the doc Issues and PRs related to the documentations. label Sep 13, 2021
@aduh95
Copy link
Contributor

aduh95 commented Sep 13, 2021

We're using the CSS property content-visibility (https://web.dev/content-visibility/) on the HTML version of the docs so it doesn't take forever to render pages on Chromium-based browsers. One of the downside of using this property is the scroll bar makes indeed some strange jumps. It was introduced in #37301.

@mscdex
Copy link
Contributor Author

mscdex commented Sep 14, 2021

I wonder if the kind of solution described here or something similar would solve these problems?

It's very frustrating not being able to properly navigate the documentation a page at a time or by sliding the scroll bar. It seems to me like the browser should keep elements rendered after it's rendered the first time, which should solve the scrolling problem. It seems like the solution I linked to explicitly does this?

@aduh95
Copy link
Contributor

aduh95 commented Sep 14, 2021

Yep that looks promising indeed. Would you be interested in sending a PR? On the mean time (and I know this is not a satisfactory answer to that issue, more a temporary workaround), you may find Firefox more suitable to browse the docs as they haven't implemented content-visibility and they also able to render large pages in a reasonable time.

@himself65
Copy link
Member

It works fine on macOS Chrome

@szmarczak
Copy link
Member

I think this also causes an issue if you try to open any URL with a hash in a new tab. It doesn't scroll to the desired method / property.

@saltybuckets
Copy link

saltybuckets commented Sep 25, 2021

I am working on this rn

saltybuckets added a commit to saltybuckets/node that referenced this issue Sep 25, 2021
This pr fixes the jittery scrolling behavior introduced in
nodejs#37301

fix:nodejs#40099
refs:https://infrequently.org/2020/12/resize-resilient-deferred-rendering/
@Trott Trott added the confirmed-bug Issues with confirmed bugs. label Nov 14, 2021
@noinkling
Copy link

noinkling commented Jan 15, 2022

I hate to pile on but this issue has made browsing the API docs an exercise in frustration for a while now. The scrollbar jumps all over the place when trying to drag it, as well as the yellow indicators when you do a ctrl+f. I never end up in the correct place when following an anchor link.

I'm surprised there haven't been more reports after all this time, surely it's more than just a handful of us affected? Chrome on Windows here. Firefox seems fine as mentioned.

@ovflowd
Copy link
Member

ovflowd commented May 5, 2023

@mscdex, for reading the current open Issue, for me, the takes are the following:

  • The original content-visibility was introduced to reduce the re-paint on resizing of pages or other shenanigans
  • It also had the benefit of a smaller initial rendering footprint

Then there's the discussion about if it should be removed:

  • It would fix anchor link positioning behaviour, which is an important aspect of the experience
  • It would allow scrolling behaviour to be respected

There's, of course, the approach of doing some JavaScript spaghetti, as I saw in some random blog post. Still, I'm severely against having arbitrary JavaScript code that tries to "fix" the web (by inherently trying to change how certain behaviours happen).

What I imagine as an "ideal" solution for now:

  • Remove the content-visibility API.
    • 99th percentile of devices nowadays are efficient enough to render even the longest pages hassle-free. Of course, constant resizes of the page on desktop browsers will increase the CPU heap during the resize by a lot, but that's an OKAY outcome. People will only sometimes resize their pages; that is an edge case, imho.
  • Add CSS grids for the two columns.
    • This page was probably done before the time of CSS grids.
    • It would allow us to remove a few margin-padding hacks
    • It would fix the width issue that happens due to content-visibility being removed
    • Enforces the maximum width of the second column and respects the page size

Regarding modern technologies:

Modern browsers and rendering engines are already smart enough to offload computing from complex parts.

I did some synthetic loads on the current API website with content-visibility disabled and requested Chrome to profile the page. I tried to do intensive scrolling behaviour to check for bottlenecks. I saw no significant bottlenecks, CPU-heap increase or significant delays/increase on the repainting/re-rendering of the page.

Moreover, disabling content-visibility shouldn't be a problem. I tried the same behaviour on an iPhone by using remote DevTools to disable those CSS rules. Didn't see any noticeable lag.

Of course, some real old phones or hardware would struggle a little. But given that technology is ever-evolving, I doubt someone would consistently open our API docs on a tiny smartphone from 7 years ago...

I feel inclined toward the approach/solution I mentioned above.

@aduh95
Copy link
Contributor

aduh95 commented May 5, 2023

  • 99th percentile of devices nowadays are efficient enough to render even the longest pages hassle-free.

I wonder where you get that figure from, in my experience loading the all.html on a Chromium browser was a real struggle for my 2019 MBP, and I’m pretty sure that’s not the lowest spec you’ll find out there. But maybe Google or Microsoft fixed that since I made the measurements reported in #37301 (comment)

@ovflowd
Copy link
Member

ovflowd commented May 5, 2023

Well, the percentile is based on Browserslists on which browsers supporting supporting the 95th percentile. (https://browserslist.dev/?q=Y292ZXIgOTUl)

And sorry, I meant the 95th percentile not the 99th.

I can at least on all my devices open it perfectly without any issues. Using Chrome DevTools to mimic hardware constrained situations it also seems to work fine.

Of course to a certain degree I'm doing an opinionated guess.

@aduh95
Copy link
Contributor

aduh95 commented May 5, 2023

To clarify, do you mean that building the docs locally commenting out the content-visibility from the CSS has no effect on the rendering time? That’s surprising to say the least, but a very good news if true (I can’t confirm, I’m away from computer). Can you share the rendering time on both versions please?

@ovflowd
Copy link
Member

ovflowd commented May 5, 2023

To clarify, do you mean that building the docs locally commenting out the content-visibility from the CSS has no effect on the rendering time?

No need to rebuild the docs; just disable the property directly on the browser.

That’s surprising to say the least, but a very good news if true (I can’t confirm, I’m away from computer). Can you share the rendering time on both versions please?

I'm not sure I can give you a reliable rendering time or statistics as they're bound to my computer. But by even doing a 6x CPU slow down and checking GPU raster it sounds OK. Feel free to check by yourself.

@aduh95
Copy link
Contributor

aduh95 commented May 5, 2023

No need to rebuild the docs; just disable the property directly on the browser.

Yes you need to rebuild the docs, because content-visibility (used to?) affect the rendering time, so disabling it from DevTools won’t help there because it won’t let you disable it until after the first rendering is done (unless I’m missing something). I can try later, as I said I’m away from the computer at the moment.

I'm not sure I can give you a reliable rendering time or statistics as they're bound to my computer

That’s OK, what I’m interested in is the difference (on the same CPU) of rendering time, absolute values don’t matter in this case.

@ovflowd
Copy link
Member

ovflowd commented May 5, 2023

Yes you need to rebuild the docs, because content-visibility (used to?) affect the rendering time, so disabling it from DevTools won’t help there because it won’t let you disable it until after the first rendering is done (unless I’m missing something). I can try later, as I said I’m away from the computer at the moment.

Well, you know you can just download all the assets, change the style.css and then run a local server 😅

I'm not on my MacBook, hence why I didn't have the tools to build the docs. But afaik just downloading a copy of the built website (Ctrl + S) + Editing the CSS files + some random simple node server should be enough.

That’s OK, what I’m interested in is the difference (on the same CPU) of rendering time, absolute values don’t matter in this case.

Well, I saw a difference of just shy ~500ms (Mostly) on the Rendering Time. Note that I started the profiling before opening the Website.

@aduh95
Copy link
Contributor

aduh95 commented May 6, 2023

OK so I've got back to my computer, and tried on Chromium 112.0.5615.137 on my M2 MBP: using content-visibility: auto makes the rendering happens 6x faster, and it takes 70% of the time to load the same page.

With content-visibility: auto: rendering takes 269ms:

image

Without content-visibility: auto: rendering takes 1732ms:

image

(I promise I didn't cherry-picked it, I run the performance profiller once on all.html for each and took a screenshot; still using a sample of 1 is not a very scientific way of comparing the two, but IMO it's really not insignificant).

@ovflowd
Copy link
Member

ovflowd commented May 6, 2023

(I promise I didn't cherry-picked it, I run the performance profiller once on all.html for each and took a screenshot; still using a sample of 1 is not a very scientific way of comparing the two, but IMO it's really not insignificant).

Yeah agreed. For me, for example, the rendering time with content-visibility costs me around 870ms, without content-visibility, I get around 1540ms. So it is also dependent on your hardware, which is fair. Regardless, we see a consistent 120% uplift in rendering time, which is fine.

But honestly speaking, I feel that in the future (with the API Docs redesign), we should eliminate the "all.html" page. It genuinely doesn't make sense to have a gigantic page containing everything simultaneously. No other Docs Page I know does this. All other pages on our API Docs, including http.html, take around 33ms to load... So, all other pages have reasonable loading times.

@aduh95
Copy link
Contributor

aduh95 commented May 6, 2023

fs.html is another huge one that you can test to see how it affects the load. We can decide that it’s fine indeed, and ask folks to use a non-Chromium browser if they hardware has a hard time loading our docs, as long as that trade off that might work for most of our visitors (is it possible to have data on that?) and we are open about making that choice.

@ovflowd
Copy link
Member

ovflowd commented May 6, 2023

I genuinely feel right now the fact that we have content-visibility is actually breaking features of the docs pages such as scrolling and anchor links.

Not to mention the redesign docs will pretty much not have this, so my 2 cents:

  • either we remove content-visibility and add the css grid
  • we just ignore this until we switch to the new redesigned docs page that is owned by the Website Team

@dholbert
Copy link
Contributor

Until this is hashed out, there's a trivial incremental improvement that we can make to mitigate some of the usability issues here (which are particularly bad/unusable in Firefox Nightly & Firefox-with-content-visibility-enabled-in-about-config). We can avoid much of the trouble by adding auto to the contain-intrinsic-size CSS declaration, which just lets the browser save the sizes that it lazily computes, once it's been forced to compute them (by scrolling an element on-screen).

See #48195 - if anyone with the ability to review/merge PRs here wouldn't mind taking a look over there, that would be much appreciated.

@KuthorX
Copy link
Contributor

KuthorX commented Oct 11, 2023

@mscdex this issue maybe resolve by pr above, could you please check it?

@tniessen
Copy link
Member

tniessen commented Jun 13, 2024

I don't know if anything changed recently, but I've noticed that many references now jump to wrong locations, which makes navigating through the docs frustrating. This happens both for anchors within the same document and across different HTML pages, and more frequently with anchors towards the bottom of long documents, which is why I assume it's some CSS gimmick that's not working well with the current version of Firefox. Also, clicking anywhere in the document or attempting to select text sometimes randomly scrolls to a new location.

@ovflowd
Copy link
Member

ovflowd commented Jun 17, 2024

I don't know if anything changed recently, but I've noticed that many references now jump to wrong locations, which makes navigating through the docs frustrating. This happens both for anchors within the same document and across different HTML pages, and more frequently with anchors towards the bottom of long documents, which is why I assume it's some CSS gimmick that's not working well with the current version of Firefox. Also, clicking anywhere in the document or attempting to select text sometimes randomly scrolls to a new location.

It is a new bug, https://issues.chromium.org/issues/333443429

@tniessen
Copy link
Member

@ovflowd FWIW, I am using Firefox :)

@aduh95
Copy link
Contributor

aduh95 commented Jun 17, 2024

I think what's new is that recent versions of Firefox support the content-visibility CSS property (unflagged in version 125) – which is unfortunate, as contrarily to Chromium, Firefox has (had) no problem rendering the very large HTML pages from our docs.

@tniessen
Copy link
Member

Yeah, I've never had any issues opening our docs until recently. Now, navigating through them is frustrating.

@tniessen tniessen changed the title doc: weird scrolling behavior in Chrome doc: weird scrolling behavior in Chrome and Firefox Jun 17, 2024
@ovflowd
Copy link
Member

ovflowd commented Jun 17, 2024

I see, so now Firefox users are actually seeing the already existing behaviour for Chrome.

Well, the only reason for such content-visibility flag to exist, is that otherwise rendering the massive all.html page would be pretty much disgraceful for devices (in terms of performance)

That simple CSS rule allows the browser to not render/process such elements until they're not visible on your viewport. This also causes the issue that the scrollbar calculation is wrong due to... well, the scrollbar at the time of load != once you start jumping/scrolling to these sections.

I need to check if we have some weird JavaScript code regarding smooth scrolling, but these are pains that, IMO, would be mitigated with the new API docs.

@tniessen
Copy link
Member

Well, the only reason for such content-visibility flag to exist, is that otherwise rendering the massive all.html page would be pretty much disgraceful for devices (in terms of performance)

If that is quite literally the only reason, can we just set the flag for all.html? I personally never use that page and we don't cross-reference it to point to specific APIs either as far as I am aware.

@ovflowd
Copy link
Member

ovflowd commented Jun 18, 2024

Well, the only reason for such content-visibility flag to exist, is that otherwise rendering the massive all.html page would be pretty much disgraceful for devices (in terms of performance)

If that is quite literally the only reason, can we just set the flag for all.html? I personally never use that page and we don't cross-reference it to point to specific APIs either as far as I am aware.

I think we could get rid of that page altogether... Which is already the goal. I unfortunately don't have the capacity at the moment to make changes on the old API docs tooling as Im writing the new one... But I assume that it shouldn't be so hard to remove the all.html from the code.

I wonder if we even care about the broken links to all.html and if we can somewhat do redirects... I wonder why that page got created to begin with.

@ovflowd
Copy link
Member

ovflowd commented Jun 18, 2024

cc @Trott or @mhdawson maybe one of you know more about the context behind it

@aduh95
Copy link
Contributor

aduh95 commented Jun 18, 2024

It’s also the case for e.g. fs.html, or any doc page that’s large enough.

@ovflowd
Copy link
Member

ovflowd commented Jun 18, 2024

It’s also the case for e.g. fs.html, or any doc page that’s large enough.

I don't think we need content-visibility on other pages, even if they are large. On nodejs.dev the experiment worked nicely and the pages rendered nicely, and that's by knowing those pages were way more complex than the current docs.

@RedYetiDev
Copy link
Member

RedYetiDev commented Jun 19, 2024

I'm just throwing this out there:

If we are redesigning the docs, is it worth it to fix these small UI bugs when it's all gonna be overhauled soon anyway?

@tniessen tniessen changed the title doc: weird scrolling behavior in Chrome and Firefox doc: weird scrolling/anchor behavior in Chrome and Firefox Jun 19, 2024
panva added a commit to panva/node that referenced this issue Jun 19, 2024
@panva
Copy link
Member

panva commented Jun 19, 2024

Well, the only reason for such content-visibility flag to exist, is that otherwise rendering the massive all.html page would be pretty much disgraceful for devices (in terms of performance)

If that is quite literally the only reason, can we just set the flag for all.html? I personally never use that page and we don't cross-reference it to point to specific APIs either as far as I am aware.

This has been annoying me for years. #53510 only applies the CSS rule to all.html.

If we are redesigning the docs, is it worth it to fix these small UI bugs when it's all gonna be overhauled soon anyway?

Well, it's a simple fix.

nodejs-github-bot pushed a commit that referenced this issue Jun 19, 2024
Refs: #40099
PR-URL: #53510
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Claudio Wunder <[email protected]>
targos pushed a commit that referenced this issue Jun 20, 2024
Refs: #40099
PR-URL: #53510
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Claudio Wunder <[email protected]>
EliphazBouye pushed a commit to EliphazBouye/node that referenced this issue Jun 20, 2024
Refs: nodejs#40099
PR-URL: nodejs#53510
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Claudio Wunder <[email protected]>
@ovflowd
Copy link
Member

ovflowd commented Jun 21, 2024

@nodejs/releasers do we have a plan to push this change out soon?

BTW closing this issue as the PR to "fix it" was merged.

@ovflowd ovflowd closed this as completed Jun 21, 2024
bmeck pushed a commit to bmeck/node that referenced this issue Jun 22, 2024
Refs: nodejs#40099
PR-URL: nodejs#53510
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Claudio Wunder <[email protected]>
marco-ippolito pushed a commit that referenced this issue Jul 19, 2024
Refs: #40099
PR-URL: #53510
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Claudio Wunder <[email protected]>
marco-ippolito pushed a commit that referenced this issue Jul 19, 2024
Refs: #40099
PR-URL: #53510
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Claudio Wunder <[email protected]>
aduh95 pushed a commit that referenced this issue Nov 7, 2024
Refs: #40099
PR-URL: #53510
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Claudio Wunder <[email protected]>
aduh95 pushed a commit that referenced this issue Nov 7, 2024
Refs: #40099
PR-URL: #53510
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Claudio Wunder <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. doc Issues and PRs related to the documentations.
Projects
None yet