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.

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

Form input name='nodeName' breaks onSubmit event handling #6284

Open
insonifi opened this issue Mar 17, 2016 · 9 comments
Open

Form input name='nodeName' breaks onSubmit event handling #6284

insonifi opened this issue Mar 17, 2016 · 9 comments

Comments

@insonifi
Copy link

It happened that I stumbled on following edge case. If you add name='nodeName' attribute to form's input, at some point of React event handling (ChangeEventPlugin.js: shouldUseChangeEvent()) it will call: elem.nodeName && elem.nodeName.toLowerCase(), but coincidentally nodeName property refers to input and invocation fails.

Here's a jsFiddle example

@zpao
Copy link
Member

zpao commented Mar 21, 2016

Ah, nice find. The issue is that elem in that code is actually window (or form) at that point and thanks to the magic of named inputs as properties on window, this is screwing up. I wonder if there are other cases like this…

@jimfb
Copy link
Contributor

jimfb commented Mar 21, 2016

@zpao That was my first thought too, in which case we could special-case window, but I don't think that's quite right.

ReactDOM.render(
  <form><input name='nodeName' /><input type='submit' /></form>,
  document.getElementById('container')
);
console.log(window.nodeName);

I think the issue occurs as we traverse the DOM hierarchy for the purposes of synthetically bubbling the event. We are hitting the form node, which is the one that has magic expando properties. It's a bit more complicated to identify form nodes when tagName and nodeName can both be overwritten by named inputs.

In shouldUseChangeEvent, we are searching for input and select nodes. Cleanest solution I can think of at the moment is to return false for any node where typeof nodeName !== 'string'. The sucky thing is that this is not a general solution and there are likely other places in the codebase where we're going to need to play whack-a-bug.

@zpao
Copy link
Member

zpao commented Mar 21, 2016

Yea, I updated my comment and mentioned the form. Otherwise window properties are due to ids, not name, whereas form properties are there for names. Maybe something with the iframes + debugger screwed me up.

@zpao
Copy link
Member

zpao commented Mar 21, 2016

So what we could do is instead of ever checking nodeName is do instanceof checks. So this code would become

function shouldUseChangeEvent(elem) {
  return (
    elem instanceof HTMLSelectElement ||
    (elem instanceof HTMLInputElement && elem.type === 'file')
  );
}

The risk is that we're in an environment that doesn't have the globals we expect. Maybe we read them from window instead & wrap potential problems in canUseDOM checks. We shouldn't hit this path at all in node anyway so I don't think it's a problem in this case, maybe in others.

@benjycui
Copy link
Contributor

benjycui commented Sep 9, 2016

The following code will cause the same error:

<input id="nodeName" />

It seems that jQuery had met the same issue and fixed, see: http://stackoverflow.com/questions/17329025/is-id-nodename-reserved-in-html5

@shaozj
Copy link

shaozj commented Jul 5, 2017

I write this code:

var form = document.createElement('form');
var input = document.createElement('input');
input.setAttribute('id', 'nodeName');
form.appendChild(input);
console.log(form);

run the code on chrome,
output: TypeError: (var).nodeName.toLowerCase is not a function
at :5:9

I think this is a browser bug.

@nanjixiong218
Copy link

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Title</title>
</head>
<body>

    <div>
        <form id="container">
            <div>
                <input type="text" name="nodeName"/>
            </div>

        </form>
    </div>
</body>
</html>

image

it maybe a bug of chrome. the input has the id or name of 'nodeName' will override the nodeName of parent form.

but it won't affect the render of page。

but in react, if the outermost layer element is the form, react will check the nodeName ,so will throw error:

t.node.nodeName.toLowerCase is not a function

image

@fritx
Copy link

fritx commented May 3, 2018

+1, same problem here.

cc @dvajs, @ant-design, @sorrycc

related: ant-design/ant-design#2950, ant-design/ant-design#2103

# error
react-dom.development.js:2586 Uncaught TypeError: elem.nodeName.toLowerCase is not a function
    at shouldUseChangeEvent (react-dom.development.js:2586)
    at Object.extractEvents (react-dom.development.js:2770)
    at extractEvents (react-dom.development.js:1072)
const formFields = [
  { input: 'text', title: '节点名称', key: 'nodeName', required: true },
  // ...
]

<Form.Item key={item.key} label={item.title}>
  {getFieldDecorator(item.key, {
    initialValue: (defaults || {})[item.key],
    rules: [{
      required: item.required,
      message: '请填写必填项',
    }],
  })(
    <Input />
  )}
</Form.Item>

Workaround: avoid naming with the keyword nodeName

const formFields = [
  // Form input name='nodeName' breaks onSubmit event handling
  // https://github.com/facebook/react/issues/6284
  { input: 'text', title: '节点名称', key: 'nodeName_fix', required: true },
  // ...
]

function toEditModel(obj) {
  const ret = _.cloneDeep(obj);
  // ...
  ['nodeName'].forEach((k) => {
    const v = ret[k];
    ret[`${k}_fix`] = v;
  });
  return ret;
}
function fromEditModel(obj) {
  const ret = _.cloneDeep(obj);
  // ...
  ['nodeName'].forEach((k) => {
    const v = ret[`${k}_fix`];
    ret[k] = v;
  });
  return ret;
}

@dustinmoorenet
Copy link

I have found a similar issue with using style as an id in a form element while trying to set inline styles on the form element.

function App() {
  return (
    <>
      <div style={{backgroundColor: 'red'}}>
        <input id="style" defaultValue="should have a red background"/>
      </div>
      <form style={{backgroundColor: 'green'}}>
        <input id="style" defaultValue="my background wont be green"/>
      </form>
      <form style={{backgroundColor: 'blue'}}>
        <input id="stylez" defaultValue="should have a blue background"/>
      </form>
    </>
  );
}

The middle form can't set it's inline styles because a child element occupies the form property style

image

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

No branches or pull requests

9 participants