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 form checkValidity(), remove vnode.dom === .activeElement from isFormAttribute() and setAttr() #2257

Closed
wants to merge 3 commits into from
Closed

Fix form checkValidity(), remove vnode.dom === .activeElement from isFormAttribute() and setAttr() #2257

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
842e223
Remove vnode.dom === .activeElement from isFormAttribute() and setAttr()
kevinkace Oct 23, 2018
2cc3c18
Revert removing vnode.dom === .activeElement in isFormAttr()
kevinkace Oct 23, 2018
ae096c5
Built assets
kevinkace Oct 24, 2018
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
4 changes: 2 additions & 2 deletions render/render.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ module.exports = function($window) {
// Only do the coercion if we're actually going to check the value.
/* eslint-disable no-implicit-coercion */
//setting input[value] to same value by typing on focused element moves cursor to end in Chrome
if ((vnode.tag === "input" || vnode.tag === "textarea") && vnode.dom.value === "" + value && vnode.dom === $doc.activeElement) return
if ((vnode.tag === "input" || vnode.tag === "textarea") && vnode.dom.value === "" + value) return
//setting select[value] to same value while having select open blinks select dropdown in Chrome
if (vnode.tag === "select" && old !== null && vnode.dom.value === "" + value) return
//setting option[value] to same value while having select open blinks select dropdown in Chrome
Expand Down Expand Up @@ -747,7 +747,7 @@ module.exports = function($window) {
}
}
function isFormAttribute(vnode, attr) {
return attr === "value" || attr === "checked" || attr === "selectedIndex" || attr === "selected" && vnode.dom === $doc.activeElement || vnode.tag === "option" && vnode.dom.parentNode === $doc.activeElement
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure removing this activeElement check makes sense? It seems to be associated to changing the selected attr. Do we know what this check was trying to achieve? I don't think it's related to #2256
Here is the old condition with parentheses to clarify it.

return (
  attr === 'value' ||
  attr === 'checked' ||
  attr === 'selectedIndex' ||
  (attr === 'selected' && vnode.dom === $doc.activeElement) ||
  (vnode.tag === 'option' && vnode.dom.parentNode === $doc.activeElement)
);

return attr === "value" || attr === "checked" || attr === "selectedIndex" || attr === "selected" || vnode.tag === "option" && vnode.dom.parentNode === $doc.activeElement
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please revert this for now and only fix the one issue with input.value? The comment in the bug regarding this was me noting I wanted to investigate (since it appeared similar in use), not that I actively wanted to change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do!

}
function isLifecycleMethod(attr) {
return attr === "oninit" || attr === "oncreate" || attr === "onupdate" || attr === "onremove" || attr === "onbeforeremove" || attr === "onbeforeupdate"
Expand Down