Rewrite media resource handling (relative path icons, web URLs)#19143
Merged
Rewrite media resource handling (relative path icons, web URLs)#19143
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
5c0ab16 to
99f3c8c
Compare
This comment has been minimized.
This comment has been minimized.
99f3c8c to
cb2daea
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
lhecker
approved these changes
Aug 5, 2025
carlos-zamora
approved these changes
Aug 5, 2025
lhecker
approved these changes
Aug 5, 2025
Member
Author
|
tried to doc it: MicrosoftDocs/terminal#875 |
carlos-zamora
pushed a commit
that referenced
this pull request
Aug 6, 2025
There were instances where changing the icon or resetting it did not result in the text box changing. Regressed in #19143
This was referenced Sep 1, 2025
michaeljolley
pushed a commit
to microsoft/PowerToys
that referenced
this pull request
Sep 3, 2025
## Summary of the Pull Request - Introduces `FontIconGlyphClassifier` for classifying emojis and symbols. - Correctly recognizes multi-codepoint glyphs (e.g., 🧙🏼♀️ *woman mage with medium-light skin tone*). - Explicitly disallows multi-glyph icons (they would overflow anyway). - Distinguishes between emojis and regular text characters (letters, numbers, symbols), since emojis are slightly larger and require different padding. - Recognizes Unicode [Variation Selectors](https://en.wikipedia.org/wiki/Variation_Selectors_(Unicode_block)) to enforce specific styles: VS15 (U+FE0E) for text style (monochrome) and VS16 (U+FE0F) for emoji style (color). This lets developers choose which variant to display. By default, characters with both representations render as text/monochrome (e.g., ▶ `\u25B6`): <img width="428" height="39" alt="image" src="https://github.com/user-attachments/assets/c5e6865f-61de-4f45-9f3a-4e15e5e5ceb8" /> - Invalid icons are displayed as a dashed circle so extension developers can spot issues, without being overly distracting if they slip into production. - Updates `IconPathConverter` to use the new classifier for improved icon handling. - Adds `SampleIconPage` to demonstrate various icon usages and classifications. - Adjusts icon alignment in `IconBox` so icons are centered. - Scales negative padding for emojis in `IconBox` with control size, fixing misalignment and clipping (noticeable in tags and the details pane hero image). - Applies negative padding to all font icons. This removes the need for classification in these cases and ensures symbols rendered below the baseline remain visible. Based on [microsoft/terminal#19143](microsoft/terminal#19143): Co-authored-by: Dustin L. Howett <[email protected]> Pictures? Pictures! <img width="1912" height="2394" alt="image" src="https://github.com/user-attachments/assets/05a16309-b658-4f21-8f9d-9a3f20db6ad8" /> Keyboard and flag/country emojis may look a bit off, but that’s how they’re actually rendered: <img width="482" height="95" alt="image" src="https://github.com/user-attachments/assets/dc7d4d0d-3dc8-4df5-9b9f-9e977e7e989f" /> <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: - #41489 - #41496 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed
DHowett
added a commit
that referenced
this pull request
Dec 2, 2025
A few features were marked "always disabled" and then enabled in dev, canary and preview. I simplified those to "always enabled" and disabled in release instead. This required changes to Generate-FeatureStaging to make it consider `WindowsInbox` a Release branding (which, honestly, it always should have been.) - Feature_DynamicSSHProfiles - Feature_ShellCompletions - Feature_SaveSnippet - Feature_QuickFix Feature_DisableWebSourceIcons was deprecated in #19143, but the XML file never got the memo.
DHowett
added a commit
that referenced
this pull request
Dec 2, 2025
A few features were marked "always disabled" and then enabled in dev, canary and preview. I simplified those to "always enabled" and disabled in release instead. This required changes to Generate-FeatureStaging to make it consider `WindowsInbox` a Release branding (which, honestly, it always should have been.) - Feature_DynamicSSHProfiles - Feature_ShellCompletions - Feature_SaveSnippet - Feature_QuickFix Feature_DisableWebSourceIcons was deprecated in #19143, but the XML file never got the memo.
DHowett
added a commit
that referenced
this pull request
Feb 27, 2026
A few features were marked "always disabled" and then enabled in dev, canary and preview. I simplified those to "always enabled" and disabled in release instead. This required changes to Generate-FeatureStaging to make it consider `WindowsInbox` a Release branding (which, honestly, it always should have been.) - Feature_DynamicSSHProfiles - Feature_ShellCompletions - Feature_SaveSnippet - Feature_QuickFix Feature_DisableWebSourceIcons was deprecated in #19143, but the XML file never got the memo. (cherry picked from commit afb4752) Service-Card-Id: PVTI_lADOAF3p4s4BBcTlzgmV4c4 Service-Version: 1.24
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.
This pull request broadly rewrites how we handle all media resources in
the Terminal settings model.
What is a media resource?
A media resource is any JSON property that refers to a file on disk,
including:
iconon profilebackgroundImageon profile (appearance)pixelShaderPathandpixelShaderImagePathon profile (appearance)iconon command and the new tab menu entriesThe last two bullet points were newly discovered during the course of
this work.
Description of Changes
In every place the settings model used to store a string for a media
path, it now stores an
IMediaResource.A media resource must be resolved before it's used. When resolved, it
can report whether it is
Ok(found, valid) and what the finalnormalized path was.
This allows the settings model to apply some new behaviors.
One of those new behaviors is resolving media paths relative to the
JSON file that referred to them. This means fragments and user settings
can now contain local images, pixel shaders and more and refer to them
by filename.
Relative path support requires us to track the path from which every
media resource "container" was read1. For "big" objects like Profile,
we track it directly in the object and for each layer. This means that
fragments updating a profile pass their relative base path into the
mix. For some of the entries such as those in
newTabMenu, we just wingit (#19191). For everything that is recursively owned by a parent that
has a path (say each Command inside an ActionMap), we pass it in from
the parent during media resolution.
During resolution, we now track exactly which layer an icon,
background image, or pixel shader path came from and read the "base
path" from only that layer. The base path is not inherited.
Another new behavior is in the handling of web and other URLs.
Canonical and a few other WSL distributors had to resort to web URLs for
icons because we did not support loading them from the package. Julia
tried to use
ms-appx://JuliaPackageNameHere/path/to/iconfor the samereason. Neither was intended, and of the two the second should have
worked but never could2.
For both
http(s?)URLs andms-appx://URLs which specify a packagename, we now strip everything except the filename. As an example...
If my fragment specifies
https://example.net/assets/foo.ico, and myfragment was loaded from
C:\Fragments, Terminal will look only atC:\Fragments\foo.ico.This works today for Julia (they put their icon in the fragment folder
hoping that one day we would support this.) It will require some work
from existing WSL distributors.
I'm told that this is similar to how XML schema documents work.
Now, icons are special. They support Emoji and Segoe Icons. This PR
adds an early pass to avoid resolving anything that looks like an
emoji.
This PR intentionally expands the heuristic definition of an emoji. It
used to only cover 1-2 code unit emoji, which prevented the use of any
empji more complicated than "man in business suite levitating."
An icon path will now be considered an emoji or symbol icon if it is
composed of a single grapheme cluster (as measured by ICU.)
This is not perfect, as it errs on the side of allowing too many
things... but each of those things is technically a single grapheme
cluster and is a perfectly legal FontIcon ;)
Profile icons are even more special than icons. They have an
additional fallback behavior which we had to preserve. When a profile
icon fails validation, or is expressly set to
null, we fall back tothe EXE specified in the command line.
Because we do this fallback during resolution, and the icon may be
inherited by any higher profile, we can only resolve it against the
commandline at the same level as the failed or nulled icon.
Therefore, if you specify
icon: nullin yourdefaultsprofile, itwill only ever resolve to
cmd.exefor any profile that inherits it(unless you change
defaults.commandline).This change expands support for the magic keywords
desktopWallpaperand
noneto all media paths (yes, evenpixelShaderPath... but also,pixelShaderImagePath!) It also expands support for environmentvariables to all of those places. Yes, we had like forty different
handlers for different types of string path. They are now uniform.
Resource Validation
Media resources which are not found are "rejected". If a rejected
resource lives in user settings, we will generate a warning and
display it.
In the future, we could detect this in the Settings UI and display a
warning inline.
Surprises
I learned that
Windows.Foundation.Uriparses file paths intofile://URIs, but does not offer you a way to get the original file path back
out. If you pass
C:\hello world,Uri.Pathwill return/C:/hello%20world. I kid you not.As a workaround, we bail out of URL handling if the
:is too close tothe start (indicating an absolute file path).
Testing
I added a narow test hook in the media resource resolver, which is
removed completely by link-time code generation. It is a real joy.
The test cases are all new and hopefully comprehensive.
TODO
Closes #19075
Closes #16295
Closes #10359 (except it doesn't support fonts)
Supersedes #16949 somewhat (
WT_SETTINGS_DIR)Refs #18679
Refs #19215 (future work)
Refs #19201 (future work)
Refs #19191 (future work)
Footnotes
We don't bother tracking where the defaults came from, because we
control everything about them. ↩
Handling a
ms-appxpath requires us to add their package to ourdependency graph for the entire duration during which the resource will
be used. For us, that could be any time (like opening the command
palette for the first time!) ↩