merge 'class' attr when both img and svg specify it #9653
Conversation
test/command/9652.md
Outdated
| <svg id="svg_b627f92299158b36552b" role="img" class="something" width="504.00pt" height="360.00pt" viewBox="0 0 504.00 360.00" class="something please-do-not-delete-me"> | ||
| </svg> |
There was a problem hiding this comment.
This doesn't seem quite right; note that there are two class attributes.
There was a problem hiding this comment.
Oh that's unfortunate. I completely missed that, thank you.
|
Would you prefer I cleanup (squash) these commits or for you to do that yourself? |
src/Text/Pandoc/SelfContained.hs
Outdated
| @@ -258,12 +258,10 @@ combineSvgAttrs svgAttrs imgAttrs = | |||
| dropPointZero t = case T.stripSuffix ".0" t of | |||
| Nothing -> t | |||
| Just t' -> t' | |||
| combinedAttrs = concat [ | |||
| imgAttrs, | |||
| combinedAttrs = replaceMaybe mergedClasses $ imgAttrs ++ | |||
There was a problem hiding this comment.
Instead of using replaceMaybe I'd suggest getting rid of that function and just added "class" to the list at ln. 264 and also use [kv | (k,v) <- imgAttrs | k /= "class"].
But isn't there another possible source of duplicated attributes? For example, suppose imgAttrs contains widdth and so does svgAttrs; won't we get two width attributes?
There was a problem hiding this comment.
Instead of using
replaceMaybeI'd suggest getting rid of that function and just added"class"to the list at ln. 264 and also use[kv | (k,v) <- imgAttrs | k /= "class"].
Thanks - my Haskell is (very visibly) rusty, and I forgot about comprehensions.
But isn't there another possible source of duplicated attributes? For example, suppose imgAttrs contains widdth and so does svgAttrs; won't we get two width attributes?
Yes, I believe we will. I had a narrow fix in mind for the class attribute since the other ones don't seem to have a natural way to merge the data. I can make further changes, but I don't know what you would prefer:
- let
imgAttrsattribute win - let
svgAttrsattribute win - issue a warning
- issue an error
The current behavior (duplicating width attribute) appears to not cause an error on either Google Chrome or Firefox, and the first attribute wins: <svg width="200" width="300"></svg> becomes a DOM element with width="200". With that said, I don't know if this is enough to justify inaction in your view (and I don't know if there's a spec that enforces this).
Closes #9652.