Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix exotic boolean attributes #2810

Closed
wants to merge 4 commits into from
Closed

Fix exotic boolean attributes #2810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fe8589f
[DOM mocks] enable setting element.style
pygy Dec 23, 2022
06cb40e
Use the style setter directly
pygy Dec 25, 2022
600a5b8
Add tests for exotic boolean attributes (download, spellcheck, transl…
pygy Dec 23, 2022
fb9d3f3
Dedicated logic for exotic boolean attributes, fix #2809
pygy Dec 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions render/render.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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" && (
Expand Down Expand Up @@ -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
}
Expand All @@ -800,10 +806,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 = ""
Expand Down
63 changes: 63 additions & 0 deletions render/tests/test-attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"}))
Expand Down
26 changes: 23 additions & 3 deletions test-utils/domMock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,13 +346,33 @@ 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 : ""
},
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)
Expand Down
125 changes: 117 additions & 8 deletions test-utils/tests/test-domMock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,16 +665,125 @@ 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.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(err instanceof Error).equals(true)
o(div.translate).equals(false)
})
})
o.spec("events", function() {
Expand Down