feat: [7877] Allow loading SVG with viewBox only #7878
Conversation
zero41120
changed the title
| * @return {PIXI.ISize} scaled viewBox size if parse success, return (0, 0) | ||
| * otherwise. | ||
| */ | ||
| static parseWidthHeightFromViewBox(svg: string, scaleTo?: ISize): ISize |
There was a problem hiding this comment.
Could you make this private?
There was a problem hiding this comment.
Also, this name is pretty verbose, why not parseViewBoxSize?
There was a problem hiding this comment.
About unit test, I had something similar to load svg inline.
However, it will not really test this particular issue since it is a Firefox issue.
When I invoke the unit test framework, it is running on chrome, by default chrome will never use this.
Any suggestion?
There was a problem hiding this comment.
Understood that this is only specific to Firefox, that's fine.
| try | ||
| { | ||
| const parser = new DOMParser(); | ||
| const svgDocument = parser.parseFromString(svg, 'image/svg+xml'); |
There was a problem hiding this comment.
My only problem with this is parsing SVG with DOMParser is that it isn't supported in IE9. We have not yet dropped support for this browser, so we need to find another approach.
There was a problem hiding this comment.
When I check on MDN it says fully supported on IE9. I don't really have access to IE9 to verify which one.
Quick google shows something about it works on IE9. Do you have recommended method to verify it on IE9?
There was a problem hiding this comment.
See IE10 for SVG only. But application/xml does work.
There was a problem hiding this comment.
Instead of parsing, would you suggest a simple RegExp to grab the viewBox value?
|
@zero41120 thanks so much for this contribution and apologies it's taken so long to get a resolution on. I had a chat with @GoodBoyDigital about this and I'm going to close this PR. This is to workaround a browser specific issue, which is to infer width/height from viewBox on Firefox. Firefox already helpfully provides a warning for this case. The easiest fix, that doesn't rely on any code changes, is to add width and height to your SVG to prevent the warning. We don't think Pixi should be handling invalid data in this case, especially when it adds a lot of complexity to the SVGResource. It's better to nudge developers to do the right thing with their assets and make sure they are valid (even if one browser has more strict requirements). If someone needs to process lots of SVGs that are formatted this way (without width/height), I'd suggest a node.js script or Webpack plugin or some other build-environment solution which can parse the SVG correctly. |
Description of change
SVGResource's error handler.viewBox's width and heightSVGResource.parseWidthHeightFromViewBox(svgString)SVGResource.parseWidthHeightFromViewBox(svgString, size)Pre-Merge Checklist
npm run lint)npm run test)