Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Fix incorrect ID generation for images in manifest #41

Merged
merged 2 commits into from
Jun 23, 2021

Conversation

bmaupin
Copy link
Owner

@bmaupin bmaupin commented Jun 22, 2021

The media name was taken as the xml id for the manifest. This
breaks XML rules when for example the image file name

  • starts with a number
  • contains spaces or colons

@bmaupin
Copy link
Owner Author

bmaupin commented Jun 22, 2021

Looks like there's one validation warning:

$ go test
Validating using EPUB version 3.2 rules.
WARNING(PKG-010): My EPUB.epub/EPUB/images/filename with space.png(-1,-1): Filename contains spaces, therefore URI escaping is necessary. Consider removing spaces from filename.

(https://github.com/bmaupin/go-epub/runs/2887794709?check_suite_focus=true)

The media name was taken as the xml id for the manifest. This
breaks XML rules when for example the image file name

* starts with a number
* contains spaces or colons
@bmaupin
Copy link
Owner Author

bmaupin commented Jun 22, 2021

I just made a change to the original commit that generates the exact same manifest IDs for testing but doesn't require any additional test files.

@bmaupin
Copy link
Owner Author

bmaupin commented Jun 22, 2021

Looks like there's one validation warning:

$ go test
Validating using EPUB version 3.2 rules.
WARNING(PKG-010): My EPUB.epub/EPUB/images/filename with space.png(-1,-1): Filename contains spaces, therefore URI escaping is necessary. Consider removing spaces from filename.

Unfortunately it doesn't look like this warning can be fixed, even by escaping the URI: w3c/epubcheck#819

But I think we could fix it just by testing the filename with spaces outside of the EPUB validation test.

This allows us to test the actual contents of the manifest in the package file, and allows us to move the filename with space test out of the EPUB validation test, which is desirable because it throws a validation warning even when the filename is properly escaped (w3c/epubcheck#819)

Also move fixXMLId closer to where it's used
@bmaupin
Copy link
Owner Author

bmaupin commented Jun 23, 2021

@pgundlach Ok, I was able to fix the validation warning by adding a new test, which has the added benefit of doing a more thorough check of the items in the package manifest. Writing it reminded me of why I don't like to write Go anymore, but it's done 😅

Thanks for your contribution and for your patience!

@bmaupin bmaupin merged commit 39c2546 into main Jun 23, 2021
@bmaupin bmaupin deleted the 14-incorrect-xml-ids branch June 23, 2021 18:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants