-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Allow xlink:href
to get removed, fix option.value
in mock
#1979
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the null
change, can you provide more detail @isiahmeadows?
@@ -376,15 +376,15 @@ o.spec("attributes", function() { | |||
|
|||
o(a.dom.value).equals("1") | |||
}) | |||
o("null becomes the empty string", function() { | |||
o("null becomes 'null'", function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this behavior desirable? I can't think of a time where you'd want to set "null"
as an attribute value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I filed #1978 (but closed it for reasons not related to this).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you feel strongly enough about it, I can fix it pretty quickly.
(FYI, the mock still needed fixed, though.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this is in conflict with #1865 which I'm currently trying to rebase. I had set up on treating null
and undefined
in vdom indiscriminately as a lack of attribute, see fa53a66#diff-0c7e3694f9865906ffe0847957a6c3a9
I'll bring it up in gitter as well for visibility.
@@ -954,11 +954,11 @@ o.spec("domMock", function() { | |||
o(select.selectedIndex).equals(1) | |||
} | |||
}) | |||
o("option.value = null is converted to the empty string", function() { | |||
o("option.value = null is converted to 'null'", function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my previous question.
The |
Allow `xlink:href` to get removed, fix `option.value` in mock
Description
Motivation and Context
Just noticed while playing around with some optimization that
xlink:*
attributes were being mishandled, but I also noticed thatoption.value
was incorrect in the mock, so I fixed it.How Has This Been Tested?
Added some new tests and ran them all as usual.
Types of changes
Checklist:
docs/change-log.md