Repository navigation
Conversation
getImageDetails() set the size of a viewBox-only SVG only if the text started with "<svg ". An SVG with an XML declaration, a comment, or leading whitespace kept the browser default size, so the filtered copy showed cropped or empty. Also require whitespace before the width, height, and viewbox attribute names, so that "stroke-width=" does not count as "width=". Fixes darkreader#16128
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.
Fixes #16128.
Problem
getImageDetails()sets the size of a viewBox-only SVG from itsviewBox. This code had three problems:<svg. An SVG with an XML declaration, a comment, or leading whitespace skipped it. Chrome then loaded the SVG at its default size (150 × 150), so the filtered copy showed cropped or empty in a small element.includes('width=')also matchedstroke-width=on the root element, so such an SVG also skipped the code.svgText.slice(5), which assumes that the root tag is at index 0.Changes
<svgfollowed by whitespace or>, and read the opening tag from that position.width,height, andviewboxattribute names.widthandheightattributes after<svgat the found position, and keep the text before it.A comment before the root element that contains
<svgtext would match first. This is rare, and a full prolog parser needs much more code.Tests
Six new tests in
tests/inject/dynamic/image-analysis.tests.ts:stroke-widthon the root element. These four fail without the change (useViewBoxisfalse, and the size is 150 × 150).npm run test:injectpassed in headless Chrome 156 (146 of 146), andnpm run lintandnpm testpassed.