Skip to content

Unexpected value for draggable "false" becomes "true" #2935

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.

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

Closed
bfanger opened this issue Jun 2, 2019 · 6 comments
Closed

Unexpected value for draggable "false" becomes "true" #2935

bfanger opened this issue Jun 2, 2019 · 6 comments
Labels

Comments

@bfanger
Copy link
Contributor

bfanger commented Jun 2, 2019

Svelte template:

<img draggable="false">

HTML output:

<img draggable="true">

Svelte REPL example

Expected output:

<img draggable="false">

A workaround is to write:

<img draggable={false}>

The cause

The svelte compiles generates img0.draggable = "false";
"false" is converted (by the browser) to true, and updates the DOM to draggable="true"

Svelte should be made aware of attributes like draggable and spellcheck and convert the string "false" to a boolean false before setting the property.

@Conduitry
Copy link
Member

@RedHatter Do you have any thoughts about what role this plays in your recent research about attributes vs properties? I can confirm that having a hyperlink in regular html with draggable="false" makes it be not draggable, but in Svelte this is failing because it's setting the property to the string "false", which the browser interprets as the boolean attribute being true. This will be unchanged after the changes in #3013 because that leaves draggable in the lookup table.

It would appear that there are different kinds of boolean attributes, as something like disabled="false" in regular html does disable an input element.

@Conduitry Conduitry added the bug label Jun 12, 2019
@RedHatter
Copy link
Contributor

There's nothing weird going on here. When a boolean attribute is set to "false" in the markup it's interpreted as a boolean since in HTML quotes don't specify a string just an attribute. When it's set to "false" from javascript, either through setAttribute or the property, the attribute is explicitly set to a string which is interpreted as true. Another note here is that setAttribute("disabled", false) also sets the attribute to a string since the prototype for setAttribute expects two strings so javascript implicitly converts the boolean to a string.

We'll need to explicitly test for "false" when setting boolean attributes.

My first thought was this.

function set_boolean_attr(node, attr, value) {
  if(value === "false" || !value) node.removeAttribute(attr)
  else node.setAttribute(attr, "")
}

But that wouldn't work with indeterminate so a better approach would probably be something like the following.

function set_boolean_prop(node, prop, value) {
  node[prop] = value !== "false" && !!value
}

@Conduitry
Copy link
Member

Conduitry commented Jun 12, 2019

What I meant was that in regular pure HTML, <a draggable="false"> was not draggable but <input disabled="false"> was disabled. The way the two boolean attributes are handled by browsers seems to differ.

So compiling <input disabled="false"> to input.disabled = "false" is correct - it's a truthy string, and the element would be disabled, as in regular HTML.

But compiling <a draggable="false"> to a.draggable = "false" is not correct. The browser apparently has special handling for the false string in this case. I'm guessing this has something to do with which boolean attributes are enabled on elements by default. Without this special handling of "false" there'd be no way to disable certain attributes from html.

@RedHatter
Copy link
Contributor

RedHatter commented Jun 12, 2019

Sorry my mistake. Yeah, my last comment was very wrong. Sorry, it's early for me I'm not quite awake yet. That part about HTML interpreting a boolean attribute set to "false" as true is just wrong.

I double checked and draggable isn't a boolean attribute. Its a enumerated attribute similar to contenteditable.

@Conduitry
Copy link
Member

Ah okay. Is spellcheck also not a boolean attribute? This ticket also says there's an issue with that.

I guess we need to do another sweep through the attributes left in in #3013 and see which additional ones should be removed.

@RedHatter
Copy link
Contributor

RedHatter commented Jun 12, 2019

Hmm... yeah looks like it. The property is defined as a boolean which I assumed meant the attribute was a boolean attribute but I guess that's not always the case. I'll double check the rest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants