Skip to content

Added application/x-font-ttf to the list of allowed media types#1614

Merged
rdeltour merged 1 commit intomainfrom
Adding-legacy-font-media-type
Sep 1, 2025
Merged

Added application/x-font-ttf to the list of allowed media types#1614
rdeltour merged 1 commit intomainfrom
Adding-legacy-font-media-type

Conversation

@iherman
Copy link
Copy Markdown
Member

@iherman iherman commented May 30, 2025

These are the possible changes following w3c/epub-specs#667 and w3c/epub-specs#2726.

I followed the advice of @mattgarrish for the files to be changed (w3c/epub-tests#307 (comment) and w3c/epub-tests#307 (comment)). Note, however, that I was not sure what is expected in the two OPF files, mostly what the value of the href attribute should be. I probably got it wrong, it certainly needs a review... But I hope that, nevertheless, this PR helps...

Note that this change is related to EPUB 3.4. I was looking for a label for 3.4, but have not found any (and I did not want to create one, I am just a guest...). I hope I got the other labels right.

@iherman iherman added type: spec The issue is related to a Specification update type: tests The issue is related to the test suite labels May 30, 2025
@mattgarrish
Copy link
Copy Markdown
Member

Sorry, I forgot to mention that the result file (resource.feature in the parent directory) also needs changing because there's an additional non-preferred media type - it's bumped from 6 to 7 info messages returned because of this addition. I've pushed a fix for that.

The file you reference in the href attribute doesn't matter in a package document-only test. Epubcheck is restricted in this mode to only verifying the package document itself. It doesn't do any further checks on whether there are actual resources associated with the document, etc.

But one oddity of adding this media type is that the test that foreign font types are exempt (you can find it at line 151 of the feature file) now passes because it was using application/x-font-ttf. We need a different media type and I assume not a completely fake one. I'll skim around and see if I can find something else.

(FYI, you can always test if your changes are good by running mvn clean install from the root directory. Epubcheck will build without errors if everything is set to go.)

@iherman
Copy link
Copy Markdown
Member Author

iherman commented May 30, 2025

Sorry, I forgot to mention that the result file (resource.feature in the parent directory) also needs changing because there's an additional non-preferred media type - it's bumped from 6 to 7 info messages returned because of this addition. I've pushed a fix for that.

Thanks

The file you reference in the href attribute doesn't matter in a package document-only test. Epubcheck is restricted in this mode to only verifying the package document itself. It doesn't do any further checks on whether there are actual resources associated with the document, etc.

But one oddity of adding this media type is that the test that foreign font types are exempt (you can find it at line 151 of the feature file) now passes because it was using application/x-font-ttf. We need a different media type and I assume not a completely fake one. I'll skim around and see if I can find something else.

(FYI, you can always test if your changes are good by running mvn clean install from the root directory. Epubcheck will build without errors if everything is set to go.)

Is mvn a java thing? I have no idea what it does... (Last time I did something with Java was, ehem, well, over 25 years ago...)

@mattgarrish
Copy link
Copy Markdown
Member

We need a different media type and I assume not a completely fake one.

The best I can find is to switch it to application/x-font-woff and replace the font file. That seems to have been a valid woff media type at one time.

Is mvn a java thing?

It's the Apache Maven build tool. I don't think it's exclusively for java, but that's how epubcheck is designed to be compiled.

@mattgarrish
Copy link
Copy Markdown
Member

There are some brief instructions on building epubcheck in the repository's readme.

@iherman
Copy link
Copy Markdown
Member Author

iherman commented Jun 3, 2025

Note: w3c/epub-specs#2726 has been merged, this PR has therefore become relevant...

@rdeltour rdeltour self-assigned this Jun 3, 2025
@rdeltour rdeltour added status: ready for review and removed type: tests The issue is related to the test suite labels Jun 3, 2025
This is a proactive implementation of something planned in EPUB 3.4.
See w3c/epub-specs#667 and w3c/epub-specs#2726

Fix #1612

Co-authored-by: Romain Deltour <rdeltour@gmail.com>
@rdeltour rdeltour added this to the v5.3.0 milestone Sep 1, 2025
@rdeltour rdeltour force-pushed the Adding-legacy-font-media-type branch from 7ba32d8 to 66e5254 Compare September 1, 2025 14:11
@rdeltour rdeltour added status: ready to merge The pull request is ready to be merged and removed status: ready for review labels Sep 1, 2025
Copy link
Copy Markdown
Member

@rdeltour rdeltour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @iherman.
Looks all good to me. I added another test and squashed+rebased the commits.

@rdeltour rdeltour changed the base branch from main to issue/1610/icu4j September 1, 2025 14:12
Base automatically changed from issue/1610/icu4j to main September 1, 2025 15:02
@rdeltour rdeltour merged commit 66e5254 into main Sep 1, 2025
6 checks passed
@rdeltour rdeltour deleted the Adding-legacy-font-media-type branch September 1, 2025 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: ready to merge The pull request is ready to be merged type: spec The issue is related to a Specification update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants