Skip to content

Navigation Menu

Sign in
Sign up

ClickOnce: improve application manifest file detection - #773

Draft
dtivel wants to merge 8 commits into
main from
dtivel/better-manifest-file-detection
Draft

ClickOnce: improve application manifest file detection #773
dtivel wants to merge 8 commits into
main from
dtivel/better-manifest-file-detection

Conversation

@dtivel

@dtivel dtivel commented Oct 11, 2024

Copy link
Copy Markdown
Collaborator

Resolve #681

This PR replaces #758.

CC @clairernovotny, @javierdlg, @jackmtpt

dtivel requested a review from a team as a code owner October 11, 2024 23:44
Comment on lines +301 to +314
// there should only be a single result here, if the file is a valid clickonce manifest.
XmlNodeList dependentAssemblies = xmlDoc.GetElementsByTagName("dependentAssembly");
if (dependentAssemblies.Count != 1)
{
Logger.LogDebug(Resources.ApplicationManifestNotFound);
return false;
}

XmlNode? node = dependentAssemblies.Item(0);
if (node is null || node.Attributes is null)
{
Logger.LogDebug(Resources.ApplicationManifestNotFound);
return false;
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

There seems to be a bug here. The comment asserts that there should only be one dependentAssembly element. However, it seems that there can be multiple.

From https://learn.microsoft.com/visualstudio/deployment/dependency-element-clickonce-deployment?view=vs-2022#elements-and-attributes:

The dependency element is required. It has no attributes. A deployment manifest can have multiple dependency elements.

This means this change would break users with multiple dependency elements. This method needs smarter application manifest identification.

{
internal interface IXmlDocumentLoader
{
XmlDocument Load(FileInfo file);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wondering why you picked XmlDocument instead of XDocument here.

Copy link
Copy Markdown

Can confirm this resolved the issue for us with two VSTO applications where manifest signing was failing.

dtivel marked this pull request as draft April 28, 2025 14:11
@dtivel dtivel added Priority:1 Work that is critical for the release, but we could probably ship without area-clickonce Related to ClickOnce signing labels Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@dlemstra dlemstra dlemstra left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

area-clickonce Related to ClickOnce signing Priority:1 Work that is critical for the release, but we could probably ship without

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Sign many applications and manifestfiles in batch for click-once

AltStyle によって変換されたページ (->オリジナル) /