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

toc.follow not work with Chinese characters #5211

Closed
4 tasks done
jeremy-feng opened this issue Mar 18, 2023 · 10 comments
Closed
4 tasks done

toc.follow not work with Chinese characters #5211

jeremy-feng opened this issue Mar 18, 2023 · 10 comments
Labels
bug Issue reports a bug resolved Issue is resolved, yet unreleased if open

Comments

@jeremy-feng
Copy link

Context

I just installed the latest insider-4.32.3. But I found that the Chinese toc is now not scrolling with page.

This issue occurs in insider-4.32.3. However, when I installed insider-4.32.0, everything became fine.

Bug description

  1. If the head text is all in English, then when I scroll to that head, the table of contents also scrolls to the position of the head. It works well.
  2. However, if the head contains any Chinese characters, then the table of contents does not automatically scroll to the position of the head.
  3. If I click on the Chinese head in the table of contents, then the url in the browser will display Chinese shortly and then immediately disappear.
  4. If I click on the same Chinese head in the table of contents again, then the url in the browser will remain.

Related links

Reproduction

chinese-toc-follow-not-working.zip

Steps to reproduce

  1. Install mkdocs-material insider 4.32.3
  2. run mkdocs serve

Browser

Chrome

Before submitting

@squidfunk squidfunk added the needs investigation Issue must be investigated by the maintainers label Mar 18, 2023
@squidfunk
Copy link
Owner

squidfunk commented Mar 19, 2023

Thanks for reporting. Same as in #5217you must use the info plugin to create the reproduction. This is explained in the issue template, as well as in our reproduction guide. This costs us and you valuable time that we cannot invest in fixing bugs or adding new features to Material for MkDocs because we have to communicate back and forth.

@squidfunk
Copy link
Owner

Okay, so turns out this is directly related to 64ca8fe in which we removed the decoding of the URI component. The problem is that setting percent_encoded: true will also encode id attributes, which I believe is a bug in Python Markdown Extensions. While I believe that the anchor should be allowed to be percent encoded, the id must not:

Bildschirmfoto 2023-03-19 um 17 18 54

Fixed in c3ba161 by reverting the changes in 64ca8fe. @Vetrenik please take #5155 upstream.

@squidfunk
Copy link
Owner

cc @facelessuser

@squidfunk squidfunk added bug Issue reports a bug resolved Issue is resolved, yet unreleased if open and removed needs investigation Issue must be investigated by the maintainers labels Mar 19, 2023
@facelessuser
Copy link
Contributor

If you think there is a bug in Pymdown Extensions, please create a detailed bug report. I'm not really sure what cases of what you are having issues with.

@squidfunk
Copy link
Owner

squidfunk commented Mar 19, 2023

Jup, the OP of #5155 should take it upstream if it is believed to be a bug. The mention was meant for you to ask if you have any thoughts from the top of your head. If not, sorry for the noise and all good from our side 😊

@facelessuser
Copy link
Contributor

So, I guess I'm not understanding what is being asked and what is being used (setup). I am assuming this is related to slugs? You do not have to enable percent_encode. The encoded options were enabled on any legacy *_encode methods, but it is advised these days to use slugify and to enable the features you want, and if percent-encoding is not wanted, it should not be enabled. If there is a particular case that is incorrect with percent-encoding, we can discuss it, but I'm not sure exactly what issue is specifically being raised.

@squidfunk
Copy link
Owner

squidfunk commented Mar 20, 2023

Specifically this configuration is causing trouble:

markdown_extensions:
  - toc:
      permalink: true
      slugify: !!python/object/apply:pymdownx.slugs.slugify
        kwds:
          case: lower
          percent_encode: true

Setting percent_encode: true will correctly encode anchors, but also id attributes. I believe this is wrong, since id attributes should not be URL-encoded (can't back it up with a spec link though), and it leads to the problem described in #5155. As said, I asked the OP of #5155 to create an issue over at Python Markdown Extensions.

@facelessuser
Copy link
Contributor

Okay, it seems that percent_encode should never be used and that we should remove the option if that is indeed true. Slugify simply generates the slug (the ID), whoever is using it should decide whether to percent-encode or not. I'm not sure why I was under the impression it was a good idea to allow percent encoding for slugs. I may have been using a misguided reference.

So, short answer, don't use percent_encode and I should probably change it to do nothing while deprecating the option. Anyone generating a URL from the slug should control percent encoding, not the slug generator.

@squidfunk
Copy link
Owner

squidfunk commented Mar 20, 2023

Jup, the browser handles it for us.

@squidfunk
Copy link
Owner

Released as part of 9.1.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue reports a bug resolved Issue is resolved, yet unreleased if open
Projects
None yet
Development

No branches or pull requests

3 participants