Update Drupal's default file type icons to use SVG

Hrm. The test fail seems unrelated.

Drupal\Tests\node\Functional\NodeRevisionsAll
The string "page=1" was not found anywhere in the HTML response of the current page.

I don't think issues like this that swap out image file references in CSS can even really be verified with automated tests anyway. Sending to NR.

Add the new high-resolution SVGs files alongside existing low-resolution PNGs in all themes shipped with Drupal core. Do not remove old PNG files from the codebase. Add cleanup task #3452493: [12.x] Remove images that have been replaced in core.

I'm having second thoughts about this in the context of the StarterKit theme. Certainly for Umami and Claro themes, we shouldnt remove the PNGs. However the StarterKit theme, when used as intended (if I understand correctly), is supposed to be a kind of "line in the sand" or fork of the codebase and a snapshot in time. This implies to me that we could easily remove the old PNG files and then any new themes created from the Starterkit would have a clean slate and no duplicitous files. Tech debt free!

My apologies for starting from the wrong branch. I recreated the same existing MR branch name locally, cherry-picked the existing commits, and force-pushed to the existing MR. Back to NR.

MR has been updated to 11.x.

I've rebased the MR on the latest 11.x in an attempt to clear unrelated CI failures...

Isn't this kind of micro-optimization mostly obviated by gzip or brotli? I'm happy to be wrong, but these savings seem negligible in practice—especially considering that Drupal core’s own HTML output doesn’t aggressively strip whitespace, newlines, or indentation either.

Also worth noting: running SVGs through optimizers like SVGO can make them harder to review and maintain. For example,

path="M11 0L14 3V16H2V0H11ZM3 15H13V4H10V1H3V15ZM11 3H12.5L11 1.5V3Z"

becomes

path="m11 0 3 3v13H2V0zM3 15h10V4h-3V1H3zm8-12h1.5L11 1.5z",

which is more compact but much less readable for quick diffs or manual review.

It would be nice if there was an automated solution to always minify SVGs in core in the same way, because different tools optimize in different ways, using different default options, often stripping out important attributes. Applications that generate SVGs also generate their own different ways of structuring the SVG, so every time you pull an "optimized" file into an SVG editor, and save it again, it may make unrelated changes on the roundtrip back to optimized SVG. Therefore, the SVGs require manual edits after optimization to restore settings. (In the example in #16 focusable="false" and the width and height attributes got removed, which will hinder future proofing the SVGs in core for inlining them server-side in the HTML).

I think what is missing here is:

1) clear agreement to treat SVGs as implementation assets as opposed to hand-curated visual code.
2) a single CLI tool to optimize assets and a well-defined list of options to ensure SVGs are optimized in the same way every time.
3) (ideally), CI testing tool to balk when required attributes are missing, or when forbidden attributes are used (eg id).

All this feels somewhat outside the scope of this issue, and implies a much larger conversation. For now, I opted for hand crafted SVG code that is structured in a way that optimizes for Brotli compression, while retaining readability and ensuring inlineability in the future. If optimization becomes a big deal it would be interesting to compare apples with apples and see what the actual Brotli / gzipped size difference would be both with and without a proper optimization that leaves the required attributes in place.

There are before/after screenshots in the IS, from the raster PNG to the SVG. The approach taken on this issue is exactly the same approach taken on the loading and throbber icon issues, whereby we're not really introducing redesign, we're simply updating the format of the existing files rom raster to vector, so as not to have to spend time bikeshedding redesign, and getting this into core in the next immediate major release.

Splitting these apart into separate issues feels like it would open the door for more cat herding.

I propose keeping this on a single issue, and using the following steps to take before/after screenshots (untested):

1. Install Drupal with the Standard profile.
2. Add a multi-cardinality file field to the Article content type that allows upload of each of the file mime types.
3. Configure field display to display the generic file widget.
4. Create a node and upload a bunch of files with different file extensions. The files could be zero byte and just be the correct file extension.
5. Take before/after screenshots of the rendered node with attached files.

There would be some amount of iteration, testing, and exploration to ensure we establish a definitive list of file extensions that map to each mime type.

@Smusgrave: Thanks for bringing this up.

• In D7 and D8 these files were located in core's /modules/file/icons/ folder.
• In D9 the files moved to various folder locations, and ultimately ended up duplicated across a bunch of core themes but always maintaining a subfolder structure of `images/classy/icons` as a way to indicate that the "source of truth" for these files was from the classy theme which was being removed from core in D10. (See Umami images/classy/README.txt)
• In the current MR for D11, I opted for keeping the touch points as light as possible so that themers can easily get their starterkit themes up-to-date on a new core version by effectively downloading a zip folder of SVGs, swapping ".png" for ".svg" in their theme CSS and done.
• If we're talking about restructuring the folders, maybe that frustratingly turns this into a D12 issue, since we're not supposed to be removing the old images, since they're kind of unofficially an "api" of sorts.

It does indeed look like the `classy` folder could be removed in D11, since referencing classy is no longer appropriate in core as of D10. I'm just not sure moving them into the umami/images/svg/ folder is entirely the right thing either. For one, the svg subfolder could also be thought of as a transitionary folder and with this issue getting merged the `svg` folder itself becomes obviated because all of the images will be SVGs. Secondly, it makes sense to keep the file icons grouped together in a separate subfolder, versus thrown in with the rest of the lot.

If only we'd had had the foresight to just move these back into /core/modules/file/icons/ in Drupal 10 since no one really wants to spend time overriding these images in a theme, and if you do, you know what you're doing and can edit your Twig templates, theme CSS, and SVG files accordingly to do what you please. Maybe there was a valid reason to copy the files so many times into various core themes (perhaps because each theme was overriding the template HTML classnames and needed different CSS styles but this seems like an anti-pattern). Maybe now is the time to clean this up and centralize it all again. Get the HTML **right**, get the classnames right, and get the CSS right in one centralized location, so no-one needs to override this any longer. IMO, getting the HTML right means skipping ahead to inlining the SVGs directly into the Twig templates.

All that being said, I still think it makes sense to keep the scope here eyed towards incremental improvements to expedite the chances of this getting in with minimal bc breaks, and then push the additional considerations, consolidations and cleanup to follow-ups, but am looking forward to your considered feedback given wider core subsystem maintainer experience.

Thanks @longwave for the thoughtful review, the merge, and your kind words in #27. Much appreciated — and thanks to you and the other core maintainers for the drum-beat triage of the never-ending stream of “grains of sand” contributions like this one. 🙂

Pending:

1. Re: #30. no follow-up needed; the PNGs have already been added to #3452493: Remove images that have been replaced with svg files.
2. Re: #27. create follow-up to consolidate the new SVG files from disparate core themes into a more appropriate core location (file module / misc / etc., per that discussion).
3. (Optional) some small forward-looking 12.x-related CR opportunities:
   • a) “API deprecation”: PNGs removed from core themes for 12.x (tracked in #3452493: Remove images that have been replaced with svg files).
   • b) document the new SVG location in file module (requires follow-up being created in point 2), and
   • c) starterkit upgrade guide for themers with starterkit-based themes in D10/11 (based on point 3 in #23):
     

     the touch points [were kept] as light as possible so that themers can easily get their starterkit themes up-to-date on a new core version by effectively downloading a zip folder of SVGs, swapping ".png" for ".svg" in their theme CSS and done.

Point 3c feels like it could expand beyond just these images. Do we have a place where we’re tracking the various starterkit/theme update notes for an eventual CR? If not, I’m wondering if this becomes a D12 follow-up task — potentially driven by `git log` on the starterkit directory to collect the D11 lifecycle changes.