Resolve MRT-qualified visual assets in the manifest editor - #199
Open
Chiara Mooney (chiaramooney) wants to merge 9 commits into
Open
Resolve MRT-qualified visual assets in the manifest editor#199Chiara Mooney (chiaramooney) wants to merge 9 commits into
Chiara Mooney (chiaramooney) wants to merge 9 commits into
Conversation
An MSIX manifest references the unqualified asset name (Assets\Logo.png) while the files that ship are qualifier-suffixed (Logo.scale-200.png, Logo.targetsize-24_altform-unplated.png). The manifest editor checked the literal path only, so every visual asset field on a standard WinUI 3 project warned 'Image not found in package directory' and the logo preview stayed blank. Ports the WinApp CLI's MrtAssetHelper qualifier grammar and variant matching to TypeScript and uses it for the existence check: the literal file wins, then qualifier-suffixed siblings, then qualifier-folder layouts. Only a reference with no literal file and no variants still warns. A resolved variant shows an informational 'Resolved via MRT to ...' note and drives the thumbnail preview. Aspect-ratio checking now skips targetsize-N variants, which are square by definition regardless of the field. Fixes #191 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 607fbdd4-8e6c-4c50-a5d7-2c9d0556fb50
- Restrict MRT variant enumeration to the package and workspace roots so a manifest value can't drive readdirSync of arbitrary local or UNC directories. - Extract checkImagePath's branch selection into image-path-resolver.ts and cover it with unit tests; the logic previously lived inline in the webview message switch with no direct coverage. - Only offer 'Copy to Assets folder?' for a literal out-of-package file, so the prompt can no longer copy a variant and rewrite the manifest to a qualified name the user never typed. - Restore the '..' containment guard on the preview path. - Clear a stale image-path status synchronously on edit instead of leaving a resolved-MRT note under a path the user just retyped. - Pin dotted-base-name parity with the CLI, add compound qualifier-folder and sibling-vs-folder precedence tests, and note why manifest-validator.ts keeps its narrower qualifier list. - Delete only this spec's files in the shared E2E workspace. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 607fbdd4-8e6c-4c50-a5d7-2c9d0556fb50
Fold mrt-asset-helper.ts, image-path-resolver.ts and asset-dimensions.ts into a single image-utils.ts, and move getImageDimensions there from the editor provider so all image handling lives in one module. Drop the MrtResolution.variants field, which was built in production but only ever read by tests, and collapse the three-function variant ranking into one comparator with identical ordering. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 607fbdd4-8e6c-4c50-a5d7-2c9d0556fb50
- mrtNote now reports the resolved relative path instead of just the file name, so qualifier-folder hits read differently from the typed value - Cover the widened manifest-validator qualifier list with accepted and near-miss spellings - Assert the application logo preview falls back to the resolved variant - Fix E2E launch: give the test instance its own user-data dir so it does not hand off to a running VS Code, and dismiss the first-run dialog Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 607fbdd4-8e6c-4c50-a5d7-2c9d0556fb50
- Contain the workspace fallback in resolveManifestImagePath so a literal file resolved outside every workspace root is no longer reported as found. - Harden getImageDimensions: always close the fd, return null for truncated PNG headers instead of 0x0, and stop the JPEG marker walk on a zero-length segment instead of looping forever. - Preview non-exact resolutions that live outside the manifest folder via webview.asWebviewUri instead of leaving the preview blank. - Add unit coverage for isPathWithin, getImageDimensions, and the workspace containment case. - Clean up the E2E temp user-data dir in global teardown. - Correct the E2E README test counts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 607fbdd4-8e6c-4c50-a5d7-2c9d0556fb50
VSIX Build
Updated 2026-08-26 21:31:00 UTC · commit |
Replace the multi-key candidate ranking with a single-pass pick: prefer the scale-200 variant when one exists, otherwise the first match alphabetically. Drop the now-unused getVariantScale helper. Cut over-long explanatory comments down to two lines. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 607fbdd4-8e6c-4c50-a5d7-2c9d0556fb50
The fallback re-resolved a leading-".." reference against each workspace root, which by construction lands outside that root, so the containment guard always rejected it. Every reference it was meant to serve is already handled by the in-workspace branch above. Also drops an isAbsolute check that a leading ".." already rules out. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 607fbdd4-8e6c-4c50-a5d7-2c9d0556fb50
A reference resolving to a workspace folder itself is a directory, so resolveMrtAsset always returns null for it and the branch that reads inWorkspace can never fire. Containment alone is sufficient. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 607fbdd4-8e6c-4c50-a5d7-2c9d0556fb50
Resolving through an MRT variant is normal authoring, so the editor now just stops warning rather than explaining itself. The preview still repoints at the resolved variant and its caption still shows that file name. Removes the note text, the .validation-msg.info style it needed, and the MrtResolution.relativePath field that existed only to build it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 607fbdd4-8e6c-4c50-a5d7-2c9d0556fb50
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
1 participant
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 #191.
Problem
The AppxManifest custom editor warned
Image not found in package directoryfor visual assets that are actually present. MRT projects reference the unqualified asset name in the manifest (Assets\Square150x150Logo.png) while only qualifier-suffixed files ship on disk (Square150x150Logo.scale-200.png). ThecheckImagePathhandler did a literalfs.existsSyncchain, found nothing, and postedstatus: 'notFound'.Fix
Ported the WinApp CLI's
MrtAssetHelperqualifier grammar to TypeScript and made image-path resolution MRT-aware:scale-*, thentargetsize-*, then the lowest-ordered qualifier).foundwith anmrtNoteexplaining which file satisfied the reference, so the user still sees that the literal path isn't on disk.isPathWithin, so a crafted manifest path can't drive filesystem reads outside the project.Everything lives in a single new module,
src/manifest-editor/image-utils.ts, which also absorbed the pre-existinggetImageDimensionsand aspect-ratio helpers that were previously inline in the provider.Also in this branch:
webview.asWebviewUriinstead of going blank.getImageDimensionshardening found while writing its tests: the fd is now always closed, truncated PNG headers returnnullinstead of0×0, and a zero-length JPEG segment no longer causes an infinite loop.--user-data-dirand dismisses VS Code's first-run onboarding wizard. Without this, a second VS Code instance handed its command line to the already-running one and exited 0, soapp.firstWindow()failed — the E2E suite could not launch at all on a developer machine with VS Code open.Test coverage
src/test/image-utils.test.ts— qualifier grammar, variant matching, candidate ranking, path containment, image header parsing, and end-to-end path resolution including the out-of-workspace containment case.src/test/manifest-validator.test.ts— qualifier-token cases for the validator.src/test/e2e/mrt-asset-resolution.spec.ts— 7 tests driving the real editor against a fixture whose assets exist only as.scale-200variants, including the logo preview falling back to the resolved variant.1135/1135 unit tests and the E2E suite pass; eslint is clean on all changed files.
Review
This branch went through three rounds of the repo's
pr-reviewskill. Deliberate decisions that reviewers flagged and I kept:manifest-validator.tskeeps its own narrow qualifier list rather than delegating toisQualifierToken— the language-qualifier regex accepts any 2–3 letter token, so.bmp/.svg/.gifwould be misread as qualifiers.isMrtVariantNamecompares only the first dot-separated segment, so dotted base names never match their variants. This is exact parity with the C# CLI and is pinned by a test.