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

Copy button on code blocks broken in main page #50

Closed
robertodr opened this issue Dec 5, 2020 · 13 comments · Fixed by #54
Closed

Copy button on code blocks broken in main page #50

robertodr opened this issue Dec 5, 2020 · 13 comments · Fixed by #54

Comments

@robertodr
Copy link
Contributor

It seems that the copy button is broken (i.e. not rendering correctly), but only for code blocks on the main page:
https://enccs.github.io/intermediate-mpi/#graphical-and-text-conventions

@rkdarst
Copy link
Member

rkdarst commented Dec 6, 2020

After some investigation...

Looking at the source, this is the element of the copy button itself. It seems the icon is not showing, and instead the alt text is, which :

<img src="#_static/copy-button.svg" alt="Copy to clipboard">

Note the extra # on it, so the image link doesn't work. My sites don't have that # there. If I add the # on my site, I get the same behavior as you see there.

I have no idea yet where that # can come from. I downloaded that repository and built it myself, and it worked. Has it always been this way? I notice this directly has a code-block inside of a signature class, and I couldn't find any other examples of that. Could you try varying different things and see if you can find any other patterns of it occuring. Does it happen when building locally?

My current hope is there was some bug in some upstream version of libraries, which is fixed now, so this will go away by itself...

@robertodr
Copy link
Contributor Author

I have tried to move the code block outside the signature directive and it still shows the alt-text. It happens only for code blocks (or literal includes) in index.rst anywhere else in the document the icon is shown correctly. It also happens when building through the GitHub action :(

@rkdarst
Copy link
Member

rkdarst commented Dec 7, 2020 via email

@robertodr
Copy link
Contributor Author

Nope, it does not work locally either.

@rkdarst
Copy link
Member

rkdarst commented Dec 7, 2020

Update!

  • make html works on the main page
  • make dirhtml fails on the main page

Also index.rst is the only document which keeps its name: all other get renamed to index.html and put into a sub-directory, to have pretty URLs. This isn't the first error of an executablebooks project for the dirhtml builder, but I can't yet see how it could be a problem of sphinx-copybutton itself... I will investigate more.

@rkdarst
Copy link
Member

rkdarst commented Dec 7, 2020

Reply from ENCCS/intermediate-mpi#42, since I commented on the wrong issue:

A-ha! Thanks for the detective work! So if we renamed index.rst to something else it would fix the problem?

@rkdarst
Copy link
Member

rkdarst commented Dec 7, 2020

It seems like yes, but you really would want it to be called index.rst, so that the root would work. I am making another test repository to narrow down the problem (it happens with only sphinx+sphinx-copybotton, so after I narrow down some more, I will report it to sphinx-copybutton while I continue investigating)

@robertodr
Copy link
Contributor Author

Many thanks for digging!

@rkdarst
Copy link
Member

rkdarst commented Dec 7, 2020

I think it is a Sphinx problem. Debugging found here:
executablebooks/sphinx-copybutton#110

@rkdarst
Copy link
Member

rkdarst commented Dec 7, 2020

Fixes for Sphinx and sphinx_rtd_theme:

But these require an upstream release to properly fix. Next up, local hackish command to do it.

@rkdarst
Copy link
Member

rkdarst commented Dec 7, 2020

My workaround for fixing is this is currently

sed -i 's/url_root="#"/url_root=""/' _build/dirhtml/index.html || true

Let's see if it works for intermediate-mpi before I apply it here.

@robertodr
Copy link
Contributor Author

It worked! Thanks!

@rkdarst
Copy link
Member

rkdarst commented Dec 15, 2020

This is now fixed upstream in both sphinx and sphinx_rtd_theme. sphinx-book-theme is not affected. Other themes may possible need fixing, if they have copied the wrong code.

I think this workaround should be maintained for quite a while, though, since things will be slow to update everywhere, and we don't want to strongly tie to any particular versions.

Thanks for reporting this!

rkdarst added a commit that referenced this issue Dec 17, 2020
- Link to the sphinx-lesson issue as well, in the comment describing
  the workaround.
- Closes: #50
- Review: minimal check
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

Successfully merging a pull request may close this issue.

2 participants