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

EPUB: Relative URL strings must not have a query component #11598

Closed
vimalkvn opened this issue Aug 15, 2023 · 6 comments · Fixed by #11766
Closed

EPUB: Relative URL strings must not have a query component #11598

vimalkvn opened this issue Aug 15, 2023 · 6 comments · Fixed by #11766

Comments

@vimalkvn
Copy link

Describe the bug

With the addition of checksums to static files ( #11415 ), EPUBs created using make epub, fail to validate

Here is a sample output from EPUBCheck:

epubcheck build/epub/SphinxEPUBTest.epub 

Running epubcheck with arguments: build/epub/SphinxEPUBTest.epub

Validating using EPUB version 3.3 rules.
ERROR(RSC-033): build/epub/SphinxEPUBTest.epub/genindex.xhtml(8,85): \
Relative URL strings must not have a query component, \
but found one in "_static/pygments.css?v=8e8a900e".
ERROR(RSC-033): build/epub/SphinxEPUBTest.epub/genindex.xhtml(9,81): \
Relative URL strings must not have a query component, \
but found one in "_static/epub.css?v=0a53256b".

How to Reproduce

index.rst

Welcome to Sphinx EPUB Test's documentation!
============================================

.. toctree::
   :maxdepth: 2
   :caption: Contents:

conf.py

project = 'Sphinx EPUB Test'
copyright = '2023, Vimalkvn'
author = 'Vimalkvn'
release = '1'
extensions = []

templates_path = ['_templates']
exclude_patterns = []

html_theme = 'alabaster'
html_static_path = ['_static']

Environment Information

Platform:              linux; (Linux-5.4.0-155-generic-x86_64-with-glibc2.29)
Python version:        3.8.10 (default, May 26 2023, 14:05:08) 
[GCC 9.4.0])
Python implementation: CPython
Sphinx version:        7.1.2
Docutils version:      0.20.1
Jinja2 version:        3.1.2
Pygments version:      2.16.1

Sphinx extensions

No response

Additional context

No response

@picnixz
Copy link
Member

picnixz commented Aug 31, 2023

Unfortunately, we want to include them in the HTML output. So, should we strip them for EPUB files, possibly renaming the files when writing the output?

@AA-Turner any thoughts on that one?

@vimalkvn
Copy link
Author

Not sure if I'm missing something, but believe the checksums are not useful even while previewing the docs locally. Still need to use sphinx-build -E ... so CSS changes are picked up. Perhaps, not including checksums for local non-http(s) URLs might cover both these cases.

@picnixz
Copy link
Member

picnixz commented Sep 2, 2023

I am a bit rusty with HTML specs but here are some thoughts:

  • Query strings in src and href of a <script> or <link> are actually ignored when HTML pages are rendered by a browser (you can see this by observing that the HTML output contains checksums but the filenames not). The reason is because such constructions are allowed (see below).
  • In particular, it is not necessarily needed for visual purposes. However, if the content of some static files (e.g., CSS files) differs from time to time, it might be desirable to know and to be able to validate that some HTML file (e.g., index.html) was generated against a specific CSS file. If we do not include checksums, then index.html may look the same for both builds so we don't necessarily know which version it actually used (I think this is the primary reason of why checksums were included (correct me if I am wrong @AA-Turner)).

Now, in HTML, the URLs can be any non-empty URLs1 but in EPUB, the specifications restrict them to be URLs with optional fragment strings (but no query strings)2. As such, I confirm it's an issue for EPUB outputs but this should be kept in HTML outputs. I think most of the EPUB implementations use a regular HTML parser so they don't fail on that.

I never worked with the EPUB builder, so I don't know how hard it would be to specialise this behaviour.

@AA-Turner Do you plan to follow EPUB3.3 specs or deviate in this specific case to reduce the work?


Footnotes

  1. https://url.spec.whatwg.org/#url-parsing

  2. https://w3c.github.io/epub-specs/epub33/core/#sec-container-iri

@dvzrv
Copy link
Contributor

dvzrv commented Nov 23, 2023

Hi! 👋
I just ran into this as well, building an EPUB with sphinx.

In effect all hardware ereaders that I could test on so far ignore the CSS of the generated EPUB entirely (which is problematic).

Desktop software, such as calibre's ebook-viewer appears to be fine (as it uses qt's webengine for rendering).

dvzrv added a commit to dvzrv/sphinx that referenced this issue Nov 23, 2023
If the EPUB builder is detected, do not add checksums in a query
component to the URL of css and js assets.

The XHTML in EPUBs does not allow the use of query components in
relative URLs.
The checksumming of assets, introduced in
0241c58 unconditionally adds a query
component for the checksum, which breaks rendering on many enduser
devices (tested on e.g. Kindle Paperwhite 3, Kobo Aura H2O Edition 2).

Fixes sphinx-doc#11598

Signed-off-by: David Runge <dave@sleepmap.de>
dvzrv added a commit to dvzrv/sphinx that referenced this issue Nov 23, 2023
If the EPUB builder is detected, do not add checksums in a query
component to the URL of css and js assets.

The XHTML in EPUBs does not allow the use of query components in
relative URLs.
The checksumming of assets, introduced in
ae20669 unconditionally adds a query
component for the checksum, which breaks rendering on many enduser
devices (tested on e.g. Kindle Paperwhite 3, Kobo Aura H2O Edition 2).

Fixes sphinx-doc#11598

Signed-off-by: David Runge <dave@sleepmap.de>
@dvzrv
Copy link
Contributor

dvzrv commented Nov 23, 2023

@vimalkvn if you have time to test the fix proposed in #11766, that'd be ace! :)

@vimalkvn
Copy link
Author

@vimalkvn if you have time to test the fix proposed in #11766, that'd be ace! :)

That works. With this fix, I get a valid epub with the template I use. Thank you!

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

Successfully merging a pull request may close this issue.

3 participants