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

Scrolling to hash links is broken on Chrome #4216

Closed
Rich-Harris opened this issue Mar 4, 2022 · 29 comments · Fixed by #4221
Closed

Scrolling to hash links is broken on Chrome #4216

Rich-Harris opened this issue Mar 4, 2022 · 29 comments · Fixed by #4221
Labels
bug Something isn't working p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. scroll management
Milestone

Comments

@Rich-Harris
Copy link
Member

Describe the bug

Clicking a link like https://kit.svelte.dev/docs/configuration#version should take you to that part of the page. It works in Firefox...

image

...but not Chrome:

image

Reproduction

Go to https://kit.svelte.dev/docs/configuration#version in Chrome

Logs

No response

System Info

System:
    OS: macOS 12.0.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 69.61 MB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.13.1 - ~/.nvm/versions/node/v16.13.1/bin/node
    Yarn: 1.22.17 - ~/.nvm/versions/node/v16.13.1/bin/yarn
    npm: 8.1.2 - ~/.nvm/versions/node/v16.13.1/bin/npm
  Browsers:
    Chrome: 98.0.4758.109
    Firefox: 97.0.1
    Safari: 15.1
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.30 
    @sveltejs/adapter-static: next => 1.0.0-next.28 
    @sveltejs/kit: next => 1.0.0-next.291 
    svelte: ^3.46.0 => 3.46.4

Severity

annoyance

Additional Information

No response

@Rich-Harris Rich-Harris added the bug Something isn't working label Mar 4, 2022
@benmccann benmccann added this to the 1.0 milestone Mar 4, 2022
@PH4NTOMiki
Copy link
Contributor

Testing fix now

@mrkishi
Copy link
Member

mrkishi commented Mar 4, 2022

You're never going to believe it, but it was introduced in #4155 somehow.

@PH4NTOMiki
Copy link
Contributor

Makes sense, because it scrolls to the correct position temporary and then returns to 0

@PH4NTOMiki
Copy link
Contributor

It's probably something to do with that clientHeight/Width bind, some Chrome edge-case/bug, but no idea how to fix it, maybe there is some other way to do that banner without that bind

@Conduitry
Copy link
Member

Most of the more complicated stuff with the banner is no longer necessary with the shorter message on mobile that doesn't need to wrap. The whole thing should be able to be replaced with an HTML- and CSS-only banner that swaps out messages with media queries.

@PH4NTOMiki
Copy link
Contributor

#4219 it looks like it works.

@PH4NTOMiki
Copy link
Contributor

PH4NTOMiki commented Mar 4, 2022

@PH4NTOMiki
Copy link
Contributor

I think I actually figured the root cause of the issue

@PH4NTOMiki
Copy link
Contributor

Closed #4219 it was just hiding the issue, need to investigate deeper.

@Rich-Harris
Copy link
Member Author

Before this block of code, the page is correctly scrolled:

root = new Root({
target,
props: { ...result.props, stores },
hydrate: true
});

After, it's reset. So it's something to do with the hydration. Trying to create a minimal repro.

@PH4NTOMiki
Copy link
Contributor

I think the browser tries to scroll, but the max scroll(full page height) changes slightly when hydrated(maybe just 1 pixel) and that causes it, but how to solve it is beyond me.

@Rich-Harris
Copy link
Member Author

Ok I've managed to at least make a simpler repro: https://stackblitz.com/edit/sveltejs-kit-template-default-pm2sdq?file=src%2Froutes%2Ffails.svelte&terminal=dev

Visit https://sveltejs-kit-template-default-pm2sdq--3000.local.webcontainer.io/ and try going to works and fails. There's something about the combination of {@html ...} and bind:clientHeight that causes the scroll to be reset on hydration.

Rich-Harris added a commit that referenced this issue Mar 4, 2022
@benzara-tahar
Copy link

Just wanted to tell you guys, you are doing great job, I recently tried svelte (kit) and felt like am home 🚀 ( no jsx, simplicity, chrome lighthouse is very happy ..etc)
I want to help with this but am still a noob, am learning a lot reading what you're doing guys. Keep it up.

And good luck with this scrolling bug 😊

@Rich-Harris
Copy link
Member Author

Ok, so the bug isn't visible in the Svelte REPL for obvious reasons, but I think I have a basic understanding of why it's happening:

image

On line 32, in the l (cLaim) method, the children of the <main> are being detached. The HTML isn't put back until line 42 in the m (Mount) method. Between claim and mount, the addition of the resize listener is somehow causing Chrome to reset the scroll (presumably Firefox waits until work is complete before assessing whether it needs to). I'm not sure why innerHTML = html doesn't happen during claim. Will raise an issue on the Svelte repo.

So we have lots of fixes to choose from:

  • move the banner with the bind:clientHeight to below the HTML block
  • reimplement it so it doesn't need bind:clientHeight
  • add resize observer bindings to Svelte so it doesn't need to use the resize listener hack
  • populate {@html ...} tags during claim instead of mount

@PH4NTOMiki
Copy link
Contributor

PH4NTOMiki commented Mar 4, 2022

@Rich-Harris I'm on my phone now, but one solution I can think of is to save scroll position(inside client.js) to const before calling new Root
and after it just do
scrollTo(pos.x, pos.y)

I don't it's good to just fix it for the docs site because that bug could be happening on some other sites we don't know about.
That's just one of the most simplest solutions but your solutions are definitely better.

@Rich-Harris
Copy link
Member Author

Opened sveltejs/svelte#7341

Rich-Harris added a commit that referenced this issue Mar 4, 2022
* use src version locally

* workaround #4216

* reimplement banner
@PH4NTOMiki
Copy link
Contributor

@Rich-Harris It's happening again for me on the docs site, for ex. link https://kit.svelte.dev/docs/routing#advanced-routing-fallthrough-routes you posted in #4291 (comment)
Interestingly enough, it happens most of the loads, but not always.

@frederikhors
Copy link
Contributor

Interestingly enough, it happens most of the loads, but not always.

Same here! Crazy! 😄

@Rich-Harris Rich-Harris reopened this Mar 11, 2022
@PH4NTOMiki
Copy link
Contributor

@Rich-Harris any idea why this could be happening? because I have none, one workaround I can think of is to check if hash is present and scrollTop is 0, If both are true then we do document.getElementById(location.hash.slice(1)).scrollIntoView() or something like that in client.js but I understand it's bad design.

@benmccann benmccann added the p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. label Mar 17, 2022
@cliftonlabrum
Copy link

Hash links are working fine for me in Chrome Version 99.0.4844.84 on a Mac. Am I supposed to do something in particular to reproduce the issue?

https://kit.svelte.dev/docs/configuration#version

HashLink

@p-lau
Copy link

p-lau commented Apr 5, 2022

It is working fine for me...

Version 1.37.109 Chromium: 100.0.4896.60 (Official Build) (64-bit)

@winston0410
Copy link

Didn't work for me(Will attach the right version tomorrow)

@henken
Copy link

henken commented Apr 13, 2022

Works for me using Chrome Version 100.0.4896.75 (Official Build) (x86_64)

@psytrx
Copy link
Contributor

psytrx commented Apr 14, 2022

Can not reproduce the behavior when default-clicking a link from another page (external or internal), i.e. the links in the Github comments above.

However, when middle-mouse-clicking, ctrl-clicking, or pasting the URL into a new tab, the scroll offset doesn't apply just as described in the issue. The same happens when I modify a link in this issue by adding target="_blank".

I'm not sure if that's supposed to work like that, due to some browser limitations or something, so I'm not sure that helps.

Version 1.37.111 Chromium: 100.0.4896.79 (Official Build) (64-bit)

@moritzebeling
Copy link

Used to be broken in Safari as well. But seems to work from version 324

@nubpro
Copy link

nubpro commented May 16, 2022

Faced this bug while I was learning sveltekit.

Click on this link Generated Types, it doesnt scroll to that position on Chrome. But sometimes it does, very finicky.

@whataboutpereira
Copy link
Contributor

whataboutpereira commented May 26, 2022

I might have happened on a related issue - I'm finding that Chrome 102.0.5005.63 doesn't scroll to hash when the page is doing an inertial scroll from scrolling with a mouse. Hash links work only when the page has completely stopped moving from inertial scrolling.

Scrolling when inertial movement is in effect seems to be working on SvelteKit docs site though.

@dummdidumm
Copy link
Member

Can't reproduce this anymore, therefore closing this. The PR that would fix the underlying issue is sveltejs/svelte#7426

@dummdidumm dummdidumm closed this as not planned Won't fix, can't repro, duplicate, stale Aug 22, 2022
@nilslindemann
Copy link

nilslindemann commented Jun 28, 2023

Please reopen, middleclicking the link does not scroll to the hash in recent chromium based browsers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. scroll management
Projects
None yet
Development

Successfully merging a pull request may close this issue.