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

Jump to bottom

ECMAScript's OrdinaryFunctionCreate has new parameter #5500

Closed
jmdyck opened this issue Apr 30, 2020 · 7 comments · Fixed by #5514
Closed

ECMAScript's OrdinaryFunctionCreate has new parameter #5500

jmdyck opened this issue Apr 30, 2020 · 7 comments · Fixed by #5514
Assignees
@domenic
Labels
integration topic: events

Comments

@jmdyck
Copy link
Contributor

jmdyck commented Apr 30, 2020

tc39/ecma262#1870 added a _sourceText_ parameter to the abstract operation OrdinaryFunctionCreate. The HTML spec invokes that operation once, so that invocation will need to be changed, but I don't know what value it should pass to the new parameter.
@domenic
Copy link
Member

domenic commented Apr 30, 2020

Fascinating. It appears at least Chrome and Firefox mostly agree on something like

function on___eventHandlerName__(event) {
__valueOfEventHandlerBody__
}

http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=8043

however I found a case where they're inconsistent about the arguments: http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=8044

@TimothyGu
Copy link
Member

Interestingly, Chrome seems to just not support any function parameters other than event. https://software.hixie.ch/utilities/js/live-dom-viewer/saved/8046 (prints event) seems to work as expected, while https://software.hixie.ch/utilities/js/live-dom-viewer/saved/8047 (prints typeof lineno) does not. However, if you print arguments then all five arguments show up as expected.

@TimothyGu
Copy link
Member

TimothyGu commented May 1, 2020 •
edited
Loading

Even more interestingly, if one calls it after the onerror has been created, then we see that the list of arguments is as expected:

<!DOCTYPE html>
<body onerror="w(lineno)">
<script>
const
</script>
<script>
w(document.body.onerror.toString());
</script>

yields

log: 3
log: function onerror(event, source, lineno, colno, error) {
w(lineno)
}

I filed https://crbug.com/1077133 to track this.

@domenic
Copy link
Member

domenic commented May 4, 2020

It feels like the right thing to do here for the original bug is to always match the ParameterList. That's relatively easy to write up in spec.

The tests will be a bit tricky, just because I'm not sure of all the scenarios where "If eventHandler is an onerror event handler of a Window object" is true. But it shouldn't be too hard to figure out. Window itself, body in some cases, ...

@domenic domenic self-assigned this May 5, 2020
domenic added a commit that referenced this issue May 5, 2020
@domenic
Pass sourceText to OrdinaryFunctionCreate for event handlers
fbff570 
Closes #5500.
@domenic domenic mentioned this issue May 5, 2020
Merged
3 tasks
@domenic
Copy link
Member

domenic commented May 5, 2020

If eventHandler is an onerror event handler of a Window object

I'm still stuck on this phrase. (Which I probably wrote...) It seems to imply that it only works for window.onerror, not for document.body.onerror. But it sounds like from https://bugs.chromium.org/p/chromium/issues/detail?id=1077133 it should also work for document.body.onerror? Maybe that part of the spec needs to be clarified? How would we even phrase that?

It almost certainly shouldn't work for document.onerror and document.head.onerror (i.e. arbitrary HTMLElements).

@domenic
Copy link
Member

domenic commented May 5, 2020

In IRC @TimothyGu pointed out that we can capture window/body/frameset using https://html.spec.whatwg.org/multipage/webappapis.html#event-handler-target

@TimothyGu
Copy link
Member

In fact, the problematic algorithm get the current value of the event handler is only called from algorithms that have already done the "determining the target of an event handler" step, so checking whether we are talking about the Window object should be as easy as saying "eventTarget is a Window object".

domenic added a commit that referenced this issue May 8, 2020
@domenic
Pass sourceText to OrdinaryFunctionCreate for event handlers
bfe9479 
Closes #5500.

This also makes the special case more explicit by referring to the name
and eventTarget variables.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@domenic domenic

Labels
integration topic: events
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Pass sourceText to OrdinaryFunctionCreate for event handlers whatwg/html
3 participants
@domenic @TimothyGu @jmdyck