Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upPut target element, form owner, and element document on scope chain of compiled events #25534
Conversation
highfive
commented
Jan 15, 2020
|
Heads up! This PR modifies the following files:
|
pshaughn
changed the title
|
@bors-servo r+ |
|
|
|
I think #25478 is also needed before this will have much of a web compatibility improvement. Handlers that are just a blob of javascript with no function arguments usually want to see the event, not just the target. |
Put target element, form owner, and element document on scope chain of compiled events Event listeners that are created from a function object just get whatever closure the function object had, but event listeners created from a string need a special closure that acts like they were defined inside a series of `with` statements. This now happens. The existing WPT test for it, html/webappapis/scripting/events/compile-event-handler-lexical-scopes.html, also relies on other behavior we don't have, so I added an easier version of the test that doesn't involve bubbling or capturing and doesn't check any IDL properties we don't have. This new test will eventually be redundant when we have everything else the upstream test expects. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #25517 <!-- Either: --> - [X] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
|
|
This is strange. I don't see any way for this code to create InternalRawUncompiledHandler and be affected by my changes, and I don't see how self[0] could fail to exist here considering that we just got self[0] before calling into here. |
|
My wild guess here is that the "cannot allocate memory" problem also caused this error; it is not reproducing for me and it didn't happen on the Mac tests. |
|
@bors-servo retry |
Put target element, form owner, and element document on scope chain of compiled events Event listeners that are created from a function object just get whatever closure the function object had, but event listeners created from a string need a special closure that acts like they were defined inside a series of `with` statements. This now happens. The existing WPT test for it, html/webappapis/scripting/events/compile-event-handler-lexical-scopes.html, also relies on other behavior we don't have, so I added an easier version of the test that doesn't involve bubbling or capturing and doesn't check any IDL properties we don't have. This new test will eventually be redundant when we have everything else the upstream test expects. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #25517 <!-- Either: --> - [X] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
|
|
pshaughn commentedJan 15, 2020
Event listeners that are created from a function object just get whatever closure the function object had, but event listeners created from a string need a special closure that acts like they were defined inside a series of
withstatements. This now happens. The existing WPT test for it, html/webappapis/scripting/events/compile-event-handler-lexical-scopes.html, also relies on other behavior we don't have, so I added an easier version of the test that doesn't involve bubbling or capturing and doesn't check any IDL properties we don't have. This new test will eventually be redundant when we have everything else the upstream test expects../mach build -ddoes not report any errors./mach test-tidydoes not report any errors