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

Implement GlobalEventHandlers.onsubmit #5396

Closed
jdm opened this issue Mar 26, 2015 · 8 comments
Closed

Implement GlobalEventHandlers.onsubmit #5396

jdm opened this issue Mar 26, 2015 · 8 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Mar 26, 2015

From the jQuery test suite initialization:

    // Support: IE<9 (lack submit/change bubble), Firefox 17+ (lack focusin event)
    // Beware of CSP restrictions (https://developer.mozilla.org/en/Security/CSP), test/csp.php
    for ( i in { submit: true, change: true, focusin: true }) {
        div.setAttribute( eventName = "on" + i, "t" );

        support[ i + "Bubbles" ] = eventName in window || div.attributes[ eventName ].expando === false;
    }
ERROR:js::rust: Error at http://localhost:8888/test/data/jquery-1.9.1.js:1446: TypeError: div.attributes[eventName] is undefined

Needed for #5395.

@jdm
Copy link
Member Author

@jdm jdm commented Mar 26, 2015

This is as simple as adding the attributes to components/script/dom/webidls/EventHandler.webidl and adding them to the global_event_handlers macro in components/script/dom/macros.rs.

@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Mar 27, 2015

If it's just in the test suite, can we get that crap removed?

@jdm
Copy link
Member Author

@jdm jdm commented Mar 27, 2015

Well, I'm pretty sure that's the jQuery source that's being loaded and throwing that exception, since it's in jQuery-1.9.1.js.

@pikzen
Copy link
Contributor

@pikzen pikzen commented Mar 29, 2015

Could I get this assigned ?

I went ahead and applied the suggested fixes. I've got a working implementation, as in "this particular error does not happen anymore". The test suite is still broken though, as other errors happen later.

@jdm
Copy link
Member Author

@jdm jdm commented Mar 30, 2015

Yep, we'll file more issues blocking #5395 for subsequent problems discovered.

@jdm jdm added the C-assigned label Mar 30, 2015
pikzen added a commit to pikzen/servo that referenced this issue Mar 30, 2015
@jdm jdm changed the title Implement GlobalEventHandlers.onsubmit/onchange/onfocusin Implement GlobalEventHandlers.onsubmit/onfocusin Mar 30, 2015
@Ms2ger Ms2ger changed the title Implement GlobalEventHandlers.onsubmit/onfocusin Implement GlobalEventHandlers.onsubmit Mar 30, 2015
@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Mar 30, 2015

focusin is not in the spec. I filed #5454 for the real issue here.

@jdm
Copy link
Member Author

@jdm jdm commented Mar 30, 2015

@jdm
Copy link
Member Author

@jdm jdm commented Mar 30, 2015

Oh, but the event handler isn't part of GlobalEventHandlers in https://html.spec.whatwg.org/#globaleventhandlers. I see.

@Ms2ger Ms2ger closed this in 6288a46 Apr 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.