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

Error when opening with a encoded hash title #2599

Open
1 task done
Jinjiang opened this issue Sep 6, 2020 · 14 comments
Open
1 task done

Error when opening with a encoded hash title #2599

Jinjiang opened this issue Sep 6, 2020 · 14 comments

Comments

@Jinjiang
Copy link
Member

Jinjiang commented Sep 6, 2020

  • I confirm that this is an issue rather than a question.

Bug report

Steps to reproduce

Open a URL with a encoded (Chinese) hash title like:

https://ge2hg.sse.codesandbox.io/guide/#color-overrides-%E4%B8%AD%E6%96%87

Source code from codesandbox: https://codesandbox.io/s/vuepress-bug-20200906-ge2hg

What is expected?

The encoded hash name should be decoded properly I guess, and the target element should be on focus.

What is actually happening?

The target element didn't be focused on. Also, there is a JS error in the console.

it's probably because the hash name wasn't decoded before calling document.getElementById. But I couldn't dig much deeper into the code.

Other relevant information

  • Output of npx vuepress info in my VuePress project:
Environment Info:

  System:
    OS: Linux 5.4 Debian GNU/Linux 10 (buster) 10 (buster)
    CPU: (16) x64 Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz
  Binaries:
    Node: 10.21.0 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.4 - /usr/local/bin/npm
  Browsers:
    Chrome: Not Found
    Firefox: Not Found
  npmPackages:
    @vuepress/core:  1.5.4
    @vuepress/theme-default:  1.5.4
    vuepress: ^1.3.0 => 1.5.4
  npmGlobalPackages:
    vuepress: Not Found
@d-pollard
Copy link
Collaborator

Hmm, odd, this seemed to work as expected for me

@Jinjiang
Copy link
Member Author

Jinjiang commented Sep 9, 2020

I think I could provide more information.

Open a URL with a encoded (Chinese) hash title like:

https://ge2hg.sse.codesandbox.io/guide/#color-overrides-%E4%B8%AD%E6%96%87

From my side, if I start the Codesandbox project and open this link above (with the hash title) directly, I will get an error:

[Vue warn]: Error in nextTick: "SyntaxError: Failed to execute 'querySelector' on 'Document': '#color-overrides-%E4%B8%AD%E6%96%87' is not a valid selector."

at client-hook-3.js:1.

As the screenshot:

image

and also the target header wasn't on focus as well.

And I guess @bencodezen also reproduced this bug before, right? 😄 So is there any other possible way we could dig into it?

Thanks.

@d-pollard
Copy link
Collaborator

Interesting, I'll have to take a look at it again;

I couldn't repro in Firefox, but it looks like you're using chrome so will try that instead

@d-pollard
Copy link
Collaborator

After some digging, it seems like this is an issue with vue-router

Looks like this line specifically is causing the issue: https://github.com/vuejs/vue-router/blob/dev/src/util/scroll.js#L146

I was able to fix it locally, will submit a PR as soon as I can. @bencodezen - if you could possibly migrate this issue to vue router, that'd be awesome!

@bencodezen
Copy link
Member

@posva Would you or someone else have any idea what's going on here?

@posva
Copy link
Member

posva commented Sep 11, 2020

This is a limitation of the current implementation and I introduced the el option vuejs/rfcs#176 to mitigate existing issues and give full flexibility. I was planning to introduce this api in vue-router 3 and deprecate the existing one with selector. You can also use https://developer.mozilla.org/en-US/docs/Web/API/CSS/escape to escape a selector but it won't work on all browsers.

@d-pollard
Copy link
Collaborator

@posva - I was getting around the limitation using decodeURIcomponent

But even with that fix, el is still undefined on the initial load and Vue router doesn't re-try to redirect the user to the hash

Could this be a side affect of our async loading of components?

@posva
Copy link
Member

posva commented Sep 11, 2020

decodeURIComponent will decode the hash and should work with slugs, so that should cover all cases for vuepress.

I don't understand what el you are talking about. The RFC i linked is only implemented on vue router 4. scrollBehavior should still run with lazy loaded components

@d-pollard
Copy link
Collaborator

@posva

This el: https://github.com/vuejs/vue-router/blob/dev/src/util/scroll.js#L146

And we can't make that change in vuepress, it'd have to be made in vue-router I believe

@posva
Copy link
Member

posva commented Sep 11, 2020

I thought you were saying to call decodeURIComponent on to.hash before returning it in scrollBehavior at

. We cannot call decodeURIComponent at the location you shared in scroll.js. Did you mean something else?

It's important to note it will still fail in some browsers (like IE and Safari) in some scenarios (passing #%oops in IE or Safari), so it's worth wrapping the call of decode with a try-catch. This won't be necessary on Vue Router 4 because to.hash is decoded

@d-pollard
Copy link
Collaborator

@posva it might be a bit easier to possibly do a zoom call or something to show you,

but essentially, what I'm seeing is that when scrollToPosition gets called, the DOM is not populated yet with the node it needs to scroll to, so https://github.com/vuejs/vue-router/blob/dev/src/util/scroll.js#L144 ends up being undefined. Once the DOM is populated, scrollToPosition doesn't get re-called, thus the page just stays at the top, which is not the desired behavior

@posva
Copy link
Member

posva commented Sep 11, 2020

The scrollBehavior on load was introduced recently, so it might be a bug on vue-router's end. In that case, can you create a reproduction (https://jsfiddle.net/posva/9r6xhqbp/) and open a bug report so I can take a look?

@SCWR
Copy link

SCWR commented Oct 21, 2020

The final wrong position is

https://github.com/vuepress/vuepress-plugin-smooth-scroll/blob/master/src/enhanceApp.ts#L16

vuepress-plugin-smooth-scroll

This document is used here.

https://router.vuejs.org/guide/advanced/scroll-behavior.html#async-scrolling

There was no problem before because the to.hash returned by vue-router was call decodeURIComponent.

Since version 3.4.3, the uncoded hash has been returned directly.

@SCWR
Copy link

SCWR commented Oct 21, 2020

https://codesandbox.io/s/sharp-brattain-o40hs?file=/src/main.js

I tested many versions of vue-router.

vue-router not call encodeURIComponent

That way, it should be a problem with this vuepress-plugin-smooth-scroll.
Or vuepress before special treatment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants