[PATCH] shr.el: correct SVG attribute case

[PATCH] shr.el: correct SVG attribute case

[Sacha Chua]

[-- Attachment #1: Type: text/plain, Size: 1575 bytes --]

I've attached a patch for restoring the correct case of SVG attributes
parsed with libxml-parse-html-region when displayed with shr, so that
SVGs can be displayed properly. HTML attributes are case-insensitive and
converted to lowercase, but SVG attributes are case-sensitive, so things
like viewBox don't work if they're lowercased. It looks like other
implementations (ex: https://github.com/w3c/svgwg/issues/161 ) also use
the workaround of fixing attribute cases for SVGs, so I took the list
from
https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inforeign
and fixed those attributes.

Here's a small test to demonstrate the issue:

;; The following should display a full circle, not a quarter circle
(with-current-buffer (get-buffer-create "*test*")
	(erase-buffer)
	(insert "<svg width=\"100px\" height=\"100px\" viewBox=\"0 0 200 200\"><circle cx=\"100\" cy=\"100\" r=\"100\"/></svg>\n")
	(shr-insert-document (libxml-parse-html-region (point-min) (point-max)))
	(display-buffer (current-buffer)))

And for shr-correct-dom-case to make sure it's working correctly:

(ert-deftest shr-correct-dom-case ()
	(let ((case-fold-search nil))
		(should
		 (string-match
			"viewBox"
			(shr-dom-to-xml
			 (shr-correct-dom-case
				(with-temp-buffer
					(insert "<svg viewBox=\"0 0 100 100\"></svg>")
					(libxml-parse-html-region (point-min) (point-max)))))))))

There are a few more screenshots at
https://sachachua.com/blog/2024/01/patching-elfeed-and-shr-to-handle-svg-images-with-viewbox-attributes/ .

I should have copyright assignment papers on file.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-shr-Correct-SVG-attribute-case.patch --]
[-- Type: text/x-diff, Size: 3761 bytes --]

From 0a56473f5a41603ed5afc05db87b6ef35b234f7e Mon Sep 17 00:00:00 2001
From: Sacha Chua <sacha@sachachua.com>
Date: Fri, 26 Jan 2024 08:54:03 -0500
Subject: [PATCH] shr: Correct SVG attribute case

* lisp/net/shr.el (shr-correct-attribute-case): New constant.
(shr-correct-dom-case): New function to correct SVG attribute case.
(shr-tag-svg): Correct SVG attribute cases before using them.
---
 lisp/net/shr.el | 76 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 2 deletions(-)

diff --git a/lisp/net/shr.el b/lisp/net/shr.el
index 17fdffd619d..e23fc6104d2 100644
--- a/lisp/net/shr.el
+++ b/lisp/net/shr.el
@@ -1437,13 +1437,85 @@ shr-dom-print
 	(shr-dom-print elem)))))
   (insert (format "</%s>" (dom-tag dom))))
 
+(defconst shr-correct-attribute-case
+  '((attributename . attributeName)
+    (attributetype . attributeType)
+    (basefrequency . baseFrequency)
+    (baseprofile . baseProfile)
+    (calcmode . calcMode)
+    (clippathunits . clipPathUnits)
+    (diffuseconstant . diffuseConstant)
+    (edgemode . edgeMode)
+    (filterunits . filterUnits)
+    (glyphref . glyphRef)
+    (gradienttransform . gradientTransform)
+    (gradientunits . gradientUnits)
+    (kernelmatrix . kernelMatrix)
+    (kernelunitlength . kernelUnitLength)
+    (keypoints . keyPoints)
+    (keysplines . keySplines)
+    (keytimes . keyTimes)
+    (lengthadjust . lengthAdjust)
+    (limitingconeangle . limitingConeAngle)
+    (markerheight . markerHeight)
+    (markerunits . markerUnits)
+    (markerwidth . markerWidth)
+    (maskcontentunits . maskContentUnits)
+    (maskunits . maskUnits)
+    (numoctaves . numOctaves)
+    (pathlength . pathLength)
+    (patterncontentunits . patternContentUnits)
+    (patterntransform . patternTransform)
+    (patternunits . patternUnits)
+    (pointsatx . pointsAtX)
+    (pointsaty . pointsAtY)
+    (pointsatz . pointsAtZ)
+    (preservealpha . preserveAlpha)
+    (preserveaspectratio . preserveAspectRatio)
+    (primitiveunits . primitiveUnits)
+    (refx . refX)
+    (refy . refY)
+    (repeatcount . repeatCount)
+    (repeatdur . repeatDur)
+    (requiredextensions . requiredExtensions)
+    (requiredfeatures . requiredFeatures)
+    (specularconstant . specularConstant)
+    (specularexponent . specularExponent)
+    (spreadmethod . spreadMethod)
+    (startoffset . startOffset)
+    (stddeviation . stdDeviation)
+    (stitchtiles . stitchTiles)
+    (surfacescale . surfaceScale)
+    (systemlanguage . systemLanguage)
+    (tablevalues . tableValues)
+    (targetx . targetX)
+    (targety . targetY)
+    (textlength . textLength)
+    (viewbox . viewBox)
+    (viewtarget . viewTarget)
+    (xchannelselector . xChannelSelector)
+    (ychannelselector . yChannelSelector)
+    (zoomandpan . zoomAndPan))
+  "Attributes for correcting the case in SVG and MathML.
+Based on https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inforeign .")
+
+(defun shr-correct-dom-case (dom)
+  "Correct the case for SVG segments."
+  (dolist (attr (dom-attributes dom))
+    (when-let ((rep (assoc-default (car attr) shr-correct-attribute-case)))
+      (setcar attr rep)))
+  (dolist (child (dom-children dom))
+    (shr-correct-dom-case child))
+  dom)
+
 (defun shr-tag-svg (dom)
   (when (and (image-type-available-p 'svg)
 	     (not shr-inhibit-images)
              (dom-attr dom 'width)
              (dom-attr dom 'height))
-    (funcall shr-put-image-function (list (shr-dom-to-xml dom 'utf-8)
-                                          'image/svg+xml)
+    (funcall shr-put-image-function
+	     (list (shr-dom-to-xml (shr-correct-dom-case dom) 'utf-8)
+                   'image/svg+xml)
 	     "SVG Image")))
 
 (defun shr-tag-sup (dom)
-- 
2.34.1


[-- Attachment #3: Type: text/plain, Size: 27 bytes --]


What do you think?

Sacha

Re: [PATCH] shr.el: correct SVG attribute case

[Manuel Giraud via Emacs development discussions.]

Sacha Chua <sacha@sachachua.com> writes:

> I've attached a patch for restoring the correct case of SVG attributes
> parsed with libxml-parse-html-region when displayed with shr, so that
> SVGs can be displayed properly. HTML attributes are case-insensitive and
> converted to lowercase, but SVG attributes are case-sensitive, so things
> like viewBox don't work if they're lowercased. It looks like other
> implementations (ex: https://github.com/w3c/svgwg/issues/161 ) also use
> the workaround of fixing attribute cases for SVGs, so I took the list
> from
> https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inforeign
> and fixed those attributes.
>
> Here's a small test to demonstrate the issue:
>
> ;; The following should display a full circle, not a quarter circle
> (with-current-buffer (get-buffer-create "*test*")
> 	(erase-buffer)
> 	(insert "<svg width=\"100px\" height=\"100px\" viewBox=\"0 0 200 200\"><circle cx=\"100\" cy=\"100\" r=\"100\"/></svg>\n")
> 	(shr-insert-document (libxml-parse-html-region (point-min) (point-max)))
> 	(display-buffer (current-buffer)))

Hi,

FWIW, it seems that 'libxml-parse-xml-region' seems to do the right
thing.
-- 
Manuel Giraud

Re: [PATCH] shr.el: correct SVG attribute case

[Sacha Chua]

[-- Attachment #1: Type: text/plain, Size: 433 bytes --]

Hello, Manuel, all!

On Sat, Jan 27, 2024, 12:18 Manuel Giraud <manuel@ledu-giraud.fr> wrote:

> FWIW, it seems that 'libxml-parse-xml-region' seems to do the right
> thing.
>

Yes, libxml-parse-xml-region preserves case. I think shr uses
libxml-parse-html-region to try to be more forgiving of real-world HTML,
though, as documented in
https://www.gnu.org/software/emacs/manual/html_node/elisp/Parsing-HTML_002fXML.html
.

Sacha

>

[-- Attachment #2: Type: text/html, Size: 1076 bytes --]

Re: [PATCH] shr.el: correct SVG attribute case

[Manuel Giraud via Emacs development discussions.]

Sacha Chua <sacha@sachachua.com> writes:

> Hello, Manuel, all!
>
> On Sat, Jan 27, 2024, 12:18 Manuel Giraud <manuel@ledu-giraud.fr> wrote:
>
>> FWIW, it seems that 'libxml-parse-xml-region' seems to do the right
>> thing.
>>
>
> Yes, libxml-parse-xml-region preserves case. I think shr uses
> libxml-parse-html-region to try to be more forgiving of real-world HTML,
> though, as documented in
> https://www.gnu.org/software/emacs/manual/html_node/elisp/Parsing-HTML_002fXML.html

Hi Sacha,

I have looked a bit at libxml2 and it seems that the XML and HTML parser
are really two differents parsers.  And the HTML one unconditionally
does lower case tags ('htmlParseHTMLName' in HTMLparser.c).  My idea was
to see if it was possible to have an option for this but it's not the
case for the HTML parser.

So I think your patch with a correspondence table is the correct one
here.
-- 
Manuel Giraud

Re: [PATCH] shr.el: correct SVG attribute case

[Eli Zaretskii]

> From: Sacha Chua <sacha@sachachua.com>
> Date: Fri, 26 Jan 2024 09:15:15 -0500
> 
> I've attached a patch for restoring the correct case of SVG attributes
> parsed with libxml-parse-html-region when displayed with shr, so that
> SVGs can be displayed properly. HTML attributes are case-insensitive and
> converted to lowercase, but SVG attributes are case-sensitive, so things
> like viewBox don't work if they're lowercased. It looks like other
> implementations (ex: https://github.com/w3c/svgwg/issues/161 ) also use
> the workaround of fixing attribute cases for SVGs, so I took the list
> from
> https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inforeign
> and fixed those attributes.

Thanks, installed on the master branch.

Please in the future post patches to the bug tracker (using
"M-x submit-emacs-patch"), so that the discussion could be
more easily discovered if needed in the future.