From fe8589f69e8bea5f88142f1ec7325e0edcd6c067 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Date: Fri, 23 Dec 2022 22:00:05 +0100 Subject: [PATCH 1/4] [DOM mocks] enable setting element.style --- test-utils/domMock.js | 5 ++--- test-utils/tests/test-domMock.js | 13 +++++-------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/test-utils/domMock.js b/test-utils/domMock.js index f645595e0..883b2cb1e 100644 --- a/test-utils/domMock.js +++ b/test-utils/domMock.js @@ -346,9 +346,8 @@ module.exports = function(options) { get style() { return style }, - set style(_){ - // https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/style#Setting_style - throw new Error("setting element.style is not portable") + set style(value){ + this.style.cssText = value }, get className() { return this.attributes["class"] ? this.attributes["class"].value : "" diff --git a/test-utils/tests/test-domMock.js b/test-utils/tests/test-domMock.js index 269b641b7..36d3364cb 100644 --- a/test-utils/tests/test-domMock.js +++ b/test-utils/tests/test-domMock.js @@ -665,16 +665,13 @@ o.spec("domMock", function() { o(div.style.background).equals("url('/*foo*/')") }) - o("setting style throws", function () { + o("setting style updates style.cssText", function () { var div = $document.createElement("div") - var err = false - try { - div.style = "" - } catch (e) { - err = e - } + div.style = "background: red;" + + o(div.style.background).equals("red") + o(div.style.cssText).equals("background: red;") - o(err instanceof Error).equals(true) }) }) o.spec("events", function() { From 06cb40eb05e0629b68b5098cbcbddab7c9952998 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Date: Sun, 25 Dec 2022 20:32:20 +0100 Subject: [PATCH 2/4] Use the style setter directly --- render/render.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/render/render.js b/render/render.js index 75a950b27..530313809 100644 --- a/render/render.js +++ b/render/render.js @@ -800,10 +800,10 @@ module.exports = function($window) { // Styles are equivalent, do nothing. } else if (style == null) { // New style is missing, just clear it. - element.style.cssText = "" + element.style = "" } else if (typeof style !== "object") { // New style is a string, let engine deal with patching. - element.style.cssText = style + element.style = style } else if (old == null || typeof old !== "object") { // `old` is missing or a string, `style` is an object. element.style.cssText = "" From 600a5b830a399eb60536d2eb5ebe88f48c578c13 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Date: Fri, 23 Dec 2022 22:02:33 +0100 Subject: [PATCH 3/4] Add tests for exotic boolean attributes (download, spellcheck, translate), with matching mocks --- render/tests/test-attributes.js | 63 +++++++++++++++++ test-utils/domMock.js | 21 ++++++ test-utils/tests/test-domMock.js | 112 +++++++++++++++++++++++++++++++ 3 files changed, 196 insertions(+) diff --git a/render/tests/test-attributes.js b/render/tests/test-attributes.js index 1daed5e5f..c93c67e52 100644 --- a/render/tests/test-attributes.js +++ b/render/tests/test-attributes.js @@ -650,6 +650,69 @@ o.spec("attributes", function() { o(d.dom.value).equals("2") }) }) + o.spec("unusual bool attributes", function() { + o.spec("download", function() { + o("a.download created through hyperscript as a boolean attr", function() { + var a = m('a[href=#][download]') + render(root, a) + + o(a.dom.getAttribute("download")).equals("") + }) + }) + + o("spellcheck", function(){ + var div = m("[spellcheck=false]") + render(root, div) + + o(div.dom.spellcheck).equals(false) + + div = m("[spellcheck]") + render(root, div) + + o(div.dom.spellcheck).equals(true) + + div = m("[spellcheck=true]") + render(root, div) + + o(div.dom.spellcheck).equals(true) + + div = m("div", {spellcheck: true}) + render(root, div) + + o(div.dom.spellcheck).equals(true) + + div = m("div", {spellcheck: false}) + render(root, div) + + o(div.dom.spellcheck).equals(false) + }) + o("translate", function(){ + var div = m("[translate=no]") + render(root, div) + + o(div.dom.translate).equals(false) + + div = m("[translate]") + render(root, div) + + o(div.dom.translate).equals(true) + + div = m("[translate=yes]") + render(root, div) + + o(div.dom.translate).equals(true) + + div = m("div", {translate: true}) + render(root, div) + + o(div.dom.translate).equals(true) + + div = m("div", {translate: false}) + render(root, div) + + o(div.dom.translate).equals(false) + }) + }) o.spec("contenteditable throws on untrusted children", function() { o("including elements", function() { var div = m("div", {contenteditable: true}, m("script", {src: "http://evil.com"})) diff --git a/test-utils/domMock.js b/test-utils/domMock.js index 883b2cb1e..e85b8f7dd 100644 --- a/test-utils/domMock.js +++ b/test-utils/domMock.js @@ -352,6 +352,27 @@ module.exports = function(options) { get className() { return this.attributes["class"] ? this.attributes["class"].value : "" }, + get download() { + return this.hasAttribute("download") ? this.getAttribute("download") : "" + }, + set download(value) { + /*eslint-disable-next-line no-implicit-coercion*/ + this.setAttribute("download", ""+value) + }, + get spellcheck() { + return this.getAttribute("spellcheck") !== "false" + }, + set spellcheck(value) { + // we can rely on implicit bool conversion + this.setAttribute("spellcheck", value ? "true" : "false") + }, + get translate() { + return this.getAttribute("translate") !== "no" + }, + set translate(value) { + // we can rely on implicit bool conversion + this.setAttribute("translate", value ? "yes" : "no") + }, set className(value) { if (this.namespaceURI === "http://www.w3.org/2000/svg") throw new Error("Cannot set property className of SVGElement") else this.setAttribute("class", value) diff --git a/test-utils/tests/test-domMock.js b/test-utils/tests/test-domMock.js index 36d3364cb..f0caed36b 100644 --- a/test-utils/tests/test-domMock.js +++ b/test-utils/tests/test-domMock.js @@ -671,7 +671,119 @@ o.spec("domMock", function() { o(div.style.background).equals("red") o(div.style.cssText).equals("background: red;") + }) + }) + o.spec("odd exotic bool attributes", function() { + o("download when not specified as attribute beforehand", function() { + var a = $document.createElement("a") + a.setAttribute("href", "#") + + o(a.hasAttribute("download")).equals(false) + o(a.download).equals("") + + a.download = "" + + o(a.hasAttribute("download")).equals(true) + o(a.getAttribute("download")).equals("") + o(a.download).equals("") + + a.download = false + + o(a.getAttribute("download")).equals("false") + }) + o("download when specified as boolean attribute beforehand", function() { + var a = $document.createElement("a") + a.setAttribute("href", "#") + a.setAttribute("download", "") + + o(a.hasAttribute("download")).equals(true) + o(a.download).equals("") + }) + o("download when specified as string attribute beforehand", function() { + var a = $document.createElement("a") + a.setAttribute("href", "#") + a.setAttribute("download", "file-name.extension") + + o(a.hasAttribute("download")).equals(true) + o(a.download).equals("file-name.extension") + }) + o("spellcheck when not set as attribute", function() { + var div = $document.createElement("div") + + o(div.spellcheck).equals(true) + + div.spellcheck = false + + o(div.getAttribute("spellcheck")).equals("false") + + div.spellcheck = "" + + o(div.getAttribute("spellcheck")).equals("false") + + div.spellcheck = "false" + + o(div.getAttribute("spellcheck")).equals("true") + + div.spellcheck = true + + o(div.getAttribute("spellcheck")).equals("true") + }) + o("spellcheck when set as boolean attribute", function() { + var div = $document.createElement("div") + div.setAttribute("spellcheck", "") + + o(div.spellcheck).equals(true) + }) + o("spellcheck when set to 'true' as attribute", function() { + var div = $document.createElement("div") + div.setAttribute("spellcheck", "true") + + o(div.spellcheck).equals(true) + }) + o("spellcheck when set to 'false' as attribute", function() { + var div = $document.createElement("div") + div.setAttribute("spellcheck", "false") + + o(div.spellcheck).equals(false) + }) + o("translate when not set as attribute", function() { + var div = $document.createElement("div") + + o(div.translate).equals(true) + + div.translate = false + + o(div.getAttribute("translate")).equals("no") + + div.translate = "" + + o(div.getAttribute("translate")).equals("no") + + div.translate = "false" + + o(div.getAttribute("translate")).equals("yes") + + div.translate = true + + o(div.getAttribute("translate")).equals("yes") + }) + o("translate when set as boolean attribute", function() { + var div = $document.createElement("div") + div.setAttribute("translate", "") + + o(div.translate).equals(true) + }) + o("translate when set to 'true' as attribute", function() { + var div = $document.createElement("div") + div.setAttribute("translate", "yes") + + o(div.translate).equals(true) + }) + o("translate when set to 'false' as attribute", function() { + var div = $document.createElement("div") + div.setAttribute("translate", "no") + o(div.translate).equals(false) }) }) o.spec("events", function() { From fb9d3f3228f383be17883b729a646ba5ae4d5c23 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Date: Sun, 25 Dec 2022 21:03:04 +0100 Subject: [PATCH 4/4] Dedicated logic for exotic boolean attributes, fix #2809 --- render/render.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/render/render.js b/render/render.js index 530313809..e9f2721a7 100644 --- a/render/render.js +++ b/render/render.js @@ -687,7 +687,7 @@ module.exports = function($window) { if (key[0] === "o" && key[1] === "n") return updateEvent(vnode, key, value) if (key.slice(0, 6) === "xlink:") vnode.dom.setAttributeNS("http://www.w3.org/1999/xlink", key.slice(6), value) else if (key === "style") updateStyle(vnode.dom, old, value) - else if (hasPropertyKey(vnode, key, ns)) { + else if (shouldUseProp(vnode, key, value, ns)) { if (key === "value") { // Only do the coercion if we're actually going to check the value. /* eslint-disable no-implicit-coercion */ @@ -717,7 +717,7 @@ module.exports = function($window) { if (key[0] === "o" && key[1] === "n") updateEvent(vnode, key, undefined) else if (key === "style") updateStyle(vnode.dom, old, null) else if ( - hasPropertyKey(vnode, key, ns) + shouldUseProp(vnode, key, old, ns) && key !== "className" && key !== "title" // creates "null" as title && !(key === "value" && ( @@ -776,13 +776,19 @@ module.exports = function($window) { function isLifecycleMethod(attr) { return attr === "oninit" || attr === "oncreate" || attr === "onupdate" || attr === "onremove" || attr === "onbeforeremove" || attr === "onbeforeupdate" } - function hasPropertyKey(vnode, key, ns) { + // determine if a vDOM attr should be set as property or attribute + function shouldUseProp(vnode, key, value, ns) { // Filter out namespaced keys return ns === undefined && ( // If it's a custom element, just keep it. vnode.tag.indexOf("-") > -1 || vnode.attrs != null && vnode.attrs.is || // If it's a normal element, let's try to avoid a few browser bugs. key !== "href" && key !== "list" && key !== "form" && key !== "width" && key !== "height"// && key !== "type" + // `m("a[download]")` would otherwilse be set to the string "true" + && !(key === "download" && value === true) + // `m("[spellcheck=false]")` and `m("[translate=no]")` would otherwise be paradoxically truthy + && !(key === "spellcheck" && typeof value === "string") + && !(key === "translate" && typeof value === "string") // Defer the property check until *after* we check everything. ) && key in vnode.dom }