Input's form attribute and document submit handler produce an IE8 error #2332

Closed
Igor10k opened this Issue May 19, 2015 · 9 comments

Comments

Projects
None yet
4 participants
@Igor10k

Igor10k commented May 19, 2015

When you use an input with form attribute while also handling the document's submit event you get an error in IE8.

Here's an easy way to reproduce it.

<html>
<body>
  <input type="text" form="test">
  <form action="" id="test"></form>
  <script src="//code.jquery.com/jquery-1.11.3.min.js"></script>
  <script>
    $(document).on('submit', 'form', function () {});
  </script>
</body>
</html>

When you click the input in IE8 you get this error.

screen shot 2015-05-19 at 16 10 42

Here's a jsfiddle http://jsfiddle.net/1g19xt3p/1/
And since jsfiddle doesn't work in IE8 here's a direct link to the result https://jsfiddle.net/1g19xt3p/1/embedded/result/

A small research showed that here's the code responsible

jQuery.event.special.submit = {
  setup: function() {
    // skipped
    jQuery.event.add( this, "click._submit keypress._submit", function( e ) {
      var elem = e.target,
        form = jQuery.nodeName( elem, "input" ) || jQuery.nodeName( elem, "button" ) ? elem.form : undefined;
      if ( form && !jQuery._data( form, "submitBubbles" ) ) {
        // skipped
      }
    });
  },
  // skipped

The thing is elem.form returns the form attribute value in IE8 that is a String and jQuery can't get the data out of a String.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin May 19, 2015

Member

See trac-12717 and trac-14126. Also note that Microsoft will stop supporting IE8 in January 2016 so it's really not worth a lot of additional IE8 heroics at this point.

Member

dmethvin commented May 19, 2015

See trac-12717 and trac-14126. Also note that Microsoft will stop supporting IE8 in January 2016 so it's really not worth a lot of additional IE8 heroics at this point.

@dmethvin dmethvin closed this May 19, 2015

@Igor10k

This comment has been minimized.

Show comment
Hide comment
@Igor10k

Igor10k May 19, 2015

@dmethvin

I see your point, but I still think that it wouldn't hurt to change

if ( form && !jQuery._data( form, "submitBubbles" ) ) {

to

if ( form && typeof form !== "string" && !jQuery._data( form, "submitBubbles" ) ) {

or maybe even

if ( typeof form == "object" && !jQuery._data( form, "submitBubbles" ) ) {

It won't make the form attribute suddenly work in IE8. But it will allow to create such solutions without hacking into jQuery.event.special.submit. Or am I missing something?

Igor10k commented May 19, 2015

@dmethvin

I see your point, but I still think that it wouldn't hurt to change

if ( form && !jQuery._data( form, "submitBubbles" ) ) {

to

if ( form && typeof form !== "string" && !jQuery._data( form, "submitBubbles" ) ) {

or maybe even

if ( typeof form == "object" && !jQuery._data( form, "submitBubbles" ) ) {

It won't make the form attribute suddenly work in IE8. But it will allow to create such solutions without hacking into jQuery.event.special.submit. Or am I missing something?

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin May 20, 2015

Member

That might prevent an error but it won't make form elements work that have a string form property though. I'm not a big fan of silent failure. Really, IE8 doesn't support HTML5 form features and anyone who wants to support IE8 will need to deal with it.

Member

dmethvin commented May 20, 2015

That might prevent an error but it won't make form elements work that have a string form property though. I'm not a big fan of silent failure. Really, IE8 doesn't support HTML5 form features and anyone who wants to support IE8 will need to deal with it.

@Igor10k

This comment has been minimized.

Show comment
Hide comment
@Igor10k

Igor10k May 20, 2015

My reason is that when I decide to go with using form attribute I know that I'll have to implement some solution to make it work in oldIE. What I don't know at that moment is that jQuery will throw errors. So instead of just doing the planned polyfill I have to dig in the jQuery code to understand where the error comes from.

Igor10k commented May 20, 2015

My reason is that when I decide to go with using form attribute I know that I'll have to implement some solution to make it work in oldIE. What I don't know at that moment is that jQuery will throw errors. So instead of just doing the planned polyfill I have to dig in the jQuery code to understand where the error comes from.

@gibson042

This comment has been minimized.

Show comment
Hide comment
@gibson042

gibson042 May 20, 2015

Member

We should replace elem.form with jQuery.prop( elem, "form" ) so a propHook can fix it. This is exactly analogous to ae215fd#diff-89c55bb1a12d31cb4eb5d007633dc33bR12 .

Member

gibson042 commented May 20, 2015

We should replace elem.form with jQuery.prop( elem, "form" ) so a propHook can fix it. This is exactly analogous to ae215fd#diff-89c55bb1a12d31cb4eb5d007633dc33bR12 .

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin May 21, 2015

Member

Oh, true, I didn't even think about using a propHook!

Member

dmethvin commented May 21, 2015

Oh, true, I didn't even think about using a propHook!

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Aug 31, 2015

Member

@gibson042

We should replace elem.form with jQuery.prop( elem, "form" ) so a propHook can fix it.

Let me make sure if I understand you correctly - do you suggest just to switch elem.form to jQuery.prop( elem, "form" ) so that jQuery users can define a proper propHook if they need IE8 support and they need to workaround this issue?

Member

mgol commented Aug 31, 2015

@gibson042

We should replace elem.form with jQuery.prop( elem, "form" ) so a propHook can fix it.

Let me make sure if I understand you correctly - do you suggest just to switch elem.form to jQuery.prop( elem, "form" ) so that jQuery users can define a proper propHook if they need IE8 support and they need to workaround this issue?

@gibson042

This comment has been minimized.

Show comment
Hide comment
@gibson042

gibson042 Aug 31, 2015

Member

Correct.

Member

gibson042 commented Aug 31, 2015

Correct.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Sep 14, 2015

Member

Fixed in ead83b9.

Note that we only use the propHook, to make it work with IE 8 you'd have to register one that polyfills HTML5 form features.

Member

mgol commented Sep 14, 2015

Fixed in ead83b9.

Note that we only use the propHook, to make it work with IE 8 you'd have to register one that polyfills HTML5 form features.

@mgol mgol closed this Sep 14, 2015

@mgol mgol added this to the 3.0.0 milestone Sep 14, 2015

@mgol mgol removed the Has Pull Request label Nov 9, 2015

@dmethvin dmethvin modified the milestones: 1.12/2.2, 3.0.0 Jan 7, 2016

@cssmagic cssmagic referenced this issue in cssmagic/ChangeLog May 18, 2016

Open

jQuery #5

@jquery jquery locked as resolved and limited conversation to collaborators Jun 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.