readthedocs / sphinx_rtd_theme

Sphinx theme from Read the Docs
https://sphinx-rtd-theme.readthedocs.io/
MIT License
4.69k stars 1.72k forks source link

Navigation tree collapse if page link's anchor is unicode text. #495

Open 0giso opened 6 years ago

0giso commented 6 years ago

Sphinx with recommonmark does not use unicode text header as page link's anchor, just ignore them. If the header is written by unicode text only, anchor is empty like "http://example.com/foo.html#". So I use unicode_ids extension to generate unicode anchor text from header.

In that case, navigation to 3rd or below level get fall back to top level content and toc tree collapsed, because sphinx_rtd_theme try to find URI escaped anchor and fail.

Change theme.js from var anchor = encodeURI(window.location.hash); to var anchor = window.location.hash; worked for me. Is this acceptable change?

Blendify commented 6 years ago

@agjohnson you are the javascript guy

jun-ogiso commented 6 years ago

var anchor = window.location.hash;

Sorry, that worked only with Edge and IE.

This may be just a workaround, but the code below works with Edge, IE, Chrome. var anchor = decodeURI(window.location.hash);

ghost commented 6 years ago

Yes, I think that decodeURI is more correct than encodeURI. For the test string ёђѓєѕіїјљњћќўџабвг, the value of window.location.hash is:

With encodeURI(window.location.hash):

In other words, in Firefox it gets encoded twice. The alternative is to find a more cross-platform alternative to window.location.hash but decodeURI seems to work in the meantime.

Blendify commented 6 years ago

@jessetan

jessetan commented 6 years ago

@0giso do you still encounter this problem? Do you have a testcase we can see? I originally fixed this in PR #615, without checking if the original Firefox bug was still there. But just like @davidfischer in https://github.com/rtfd/sphinx_rtd_theme/pull/615#issuecomment-383160744, I no longer see any difference between Chrome and Firefox behavior on macOS. If the original Firefox bug was solved, we can close this and discard the PR.

0giso commented 6 years ago

@jessetan I am not sure #615 can fix or not this problem, but I created test case to reproduce the issue. https://github.com/0giso/testcase_for_sphinx_rtd_theme_issue495

It use sphinx_rtd_theme installed by pip.

jessetan commented 6 years ago

Thanks for the testcase. Can you explain the need for the unicode_ids extension? If I remove it, the TOC works correctly.

615 does not influence the behavior you reported and is no longer necessary because Fx now behaves identical to Chrome.

0giso commented 6 years ago

I need unicode_ids extension for non-English markdown document because if I don't use it, sphinx with recommonmark ignore non-English character in section title, then section hyper link url ends with '#' (no url fragment appended).

I updated the test case as below to show unicode_ids necessity.

Thank you.