fix: require "type/" prefix when matching MIME families in PreloadHTML#194
Merged
Conversation
assets.PreloadHTML classified MIME families with HasPrefix(mimetype, "image") and "font", so unrelated types such as "imagery/*" and "fontastic/*" were treated as images or fonts. That let non-image resources be emitted as favicons and non-font resources receive font preload metadata. Matching was also case-sensitive, missing valid types like "IMAGE/*" or "FONT/*". Match on the "image/" and "font/" prefixes case-insensitively so only genuine image/* types qualify as favicons and only genuine font/* types receive font preload handling.
Test_PreloadHTML computed its expected font preload link with strings.HasPrefix(fontMime, "font"), a case-sensitive check without the trailing slash. Since PreloadHTML now matches "font/" case-insensitively, a platform returning "FONT/woff2" would emit as="font" while the test expected the bare preload form, failing spuriously. Mirror the production classification so the expectation tracks the code on any MIME table.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #182.
Problem
assets.PreloadHTMLclassified MIME families withstrings.HasPrefix(mimetype, "image")and"font"rather than the"image/"and"font/"prefixes, so unrelated types such asimagery/*andfontastic/*were mistaken for images or fonts. This let non-image resources be emitted as favicons and non-font resources receive font preload metadata. Matching was also case-sensitive, so validly-typed-but-differently-cased resources (IMAGE/*,FONT/*) were missed.Fix
Match on the
"image/"and"font/"prefixes, lowercasing the resolved MIME type first so family matching is case-insensitive. Only genuineimage/*types now qualify as favicons, and only genuinefont/*types receiveas="font"preload handling.Tests
Added
Test_PreloadHTML_MIMEFamilyMatching, which registersimagery/*,fontastic/*,IMAGE/*andFONT/*extensions and asserts:imagery/*is not treated as an image (no favicon, noas="image").fontastic/*is not treated as a font (noas="font").IMAGE/*is recognized as an image and itsfavicon-named resource wins the favicon slot.FONT/*is recognized as a font and receivesas="font".The test fails against the unfixed source and passes with the fix.
go vetandgo test -race ./lib/assets/are clean.Acceptance criteria from the issue:
image/*types qualify as favicons.font/*types receive font preload handling.