mdn / yari

The platform code behind MDN Web Docs
Mozilla Public License 2.0
1.19k stars 502 forks source link

Scroll to URL Fragment no longer works on Chromium browsers #8049

Open NiedziolkaMichal opened 1 year ago

NiedziolkaMichal commented 1 year ago

Summary

When I enter a page with a URL fragment, the content of the page is not automatically scrolled to the identified element.

URL

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/canvas#attr-moz-opaque It happens on every page though

Reproduction steps

  1. Open Chrome
  2. Go to the specified URL
  3. Watch how the page is not scrolled down

Expected behavior

-

Actual behavior

-

Device

Desktop

Browser

Chrome

Browser version

Stable

Operating system

Windows

Screenshot

No response

Anything else?

On firefox, it works fine It might be related to #7450

Validations

tlylt commented 1 year ago

I was just checking on Chrome, it seems like perhaps only a subset of pages are affected. For example, this page's scroll behavior is still working.

But indeed the page OP mentioned: Element/canvas#attr-moz-opaque as well as some other element/xxx pages don't seem to have working scroll effects.

NiedziolkaMichal commented 1 year ago

@tlylt In my case, scroll behavior also doesn't work on the mentioned Content categories page.

tlylt commented 1 year ago

@tlylt In my case, scroll behavior also doesn't work on the mentioned Content categories page.

hmm that's strange, here's a recording from my device:

https://user-images.githubusercontent.com/41845017/216065599-9f4524f4-3645-4b41-b8fa-d521617dadb8.mp4

LeoMcA commented 1 year ago

This appears to be the result of our sidebar auto-scroll: if the browser viewport is long enough for the sidebar to not scroll, then scrolling to the anchor link on page load works. However, if viewpoint is too short and the sidebar does scroll, then scrolling to the anchor link doesn't work.

This is quite clever behaviour in Chrome, but does feel like a bit of a bug upstream, as we're not scrolling the main body of the page (where the anchor exists), just the sidebar element (where it doesn't). I can repro in the latest nightly chromium build.

We could work around this, but I get the feeling it would be messy and edge-case-ridden, something like: scrolling the sidebar on page load unless an anchor exists in the url, where we wait until a scroll event is fired on the document to scroll the sidebar.

@caugner if you agree we shouldn't attempt a workaround, I can report this upstream

caugner commented 1 year ago

I can report this upstream

@LeoMcA Yes, that's a very good idea!

LeoMcA commented 1 year ago

Reported: https://bugs.chromium.org/p/chromium/issues/detail?id=1417660

oliverdunk commented 1 year ago

Hey @LeoMcA / @caugner - I came across this and was wondering if you could point me to the code that does this in MDN? I'm trying to figure out if it's something we might be able to fix on the Chrome side (or at least, I can say if that's unlikely).

LeoMcA commented 1 year ago

Hey @oliverdunk, thanks for looking into this - this is a weird bug for sure. This is the useEffect which triggers the scroll on the sidebar (when the currently highlighted sidebar item is off screen):

https://github.com/mdn/yari/blob/4adef0f0df7886a51665d526e4c7d5919be87621/client/src/document/organisms/sidebar/index.tsx#L42-L53

Before filing the Chromium issue above, I attempted to create a more minimal test case, but couldn't reproduce the behaviour outside of MDN (obviously there's some nuance to our implementation I'm missing, perhaps its something in the React layer). e.g. this behaves as expected in Chromium when accessed like file://[...]/index.html?scroll#anchor (with both the element and whole page scrolling):

<!DOCTYPE html>
<html>
  <head>
    <style>
      html, aside {
        scroll-behavior: smooth;
      }

      aside {
        overflow: scroll;
        height: 100px;
      }

      aside div {
        margin-bottom: 200px;
      }

      h1 {
        margin-top: 100vh;
      }
    </style>
  </head>
  <body>
    <aside>
      <div>scrolling element</div>
    </aside>
    <h1 id="anchor">an anchor</h1>

    <script>
      if (location.search === "?scroll") {
        document.querySelector("aside").scrollTo({ top: 100 });
      }
    </script>
  </body>
</html>
caugner commented 4 months ago

It looks like this is being prioritized by the Chrome team: https://issues.chromium.org/issues/325081538

bsmth commented 1 month ago

It looks like a fix is landing in Chrome 130: