Svg thumbnail failed rendering

[gokcekantarci]

Summary of the Pull Request

Default value of fill-rule in style is "nonzero". There is a possibility that rendering bugs may occur due to this reason. Therefore, to eliminate these errors, it is checked whether there is a fill-rule in style and if so, it is re-applied.

https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/fill-rule

PR Checklist

• Closes: SVG not rendered correctly. #28814
• Communication: I've discussed this with core contributors already. If 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 for new binaries
  • WXS for installer for new binaries and localization folder
  • YML for CI pipeline for new test projects
  • YML for signed pipeline
• Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

I created some SVG files for testing. The first one is the "fill-rule" attribute is under style. In others it's under "polygon" and "path". When I tested it with version 0.84.1, only the one under style was broken. Therefore, I was able to solve the problem by parsing the svg style of the file and re-applying the "fill-rule" and other attributes there. The reason why I re-applied all the attributes in the style was that if there was such an error in the "fill-rule", I suspected that there might be other attributes as well. When I changed the "stroke color", "clip-rule", "stroke-width", "opacity" attributes, I did not encounter this error. However, I preserved the code this way as similar errors may occur in other attributes in the future.

0.84.1

PR

Validation Steps Performed

Check with these svg files

Test files.zip

[jaimecbernardo]

I think the implementation should be changed, since I don't understand why JavaScript is needed to get the fill-rule. We're supposed to have that already from elsewhere.
Also this fixes for the svg thumbnail but not for svg preview. What about Peek? Does it have the same issue?

[jaimecbernardo]

Looking at the solution, it doesn't seem like a safe solution. According to the sample in the documentation linked in this PR ( https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/fill-rule ) , fill-rule might be present in internal polygons and not even the svg tag. So this change might actually apply an internal fill rule to the whole SVG, since it's applying it to the svg tag.
This is likely to create errors in trying to show others files.

If what we're trying to do is restore the "fill-rule" we seem to be removing from the style of the svg, perhaps the correct solution is edit or add just the width and height instead of completely overwriting the style of the svg tag like we were doing before.
Thoughts?

[crutkas]

Is this ready for review again?

[Copilot]

Pull Request Overview

This pull request addresses an SVG thumbnail rendering bug by reapplying the fill-rule (and other style attributes) extracted from the SVG’s style attribute. The changes include removing an unused using directive, parsing the SVG content to capture style information, and adding a retry mechanism during WebView2 initialization.

Comments suppressed due to low confidence (1)

src/modules/previewpane/SvgThumbnailProvider/SvgThumbnailProvider.cs:123

• [nitpick] The variable name 'a' is ambiguous; consider renaming it to something more descriptive like 'svgViewBoxResult' for clarity.

var a = await _browser.ExecuteScriptAsync($"document.getElementsByTagName('svg')[0].viewBox;");

[lei9444]

Made some code changes, and the local test passed successfully.

updated

[lei9444]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[vanzue]

Tested, and works fine in my environment