XMLHttpRequest: Change to more consistent event invocation order - #287
Merged
Conversation
This commit changes the invocation order of events added to XMLHttpRequest, through both addEventListener() and on-handlers, to be more consistent with major browser such as Chrome / FF / IE, as well as other events already implemented in HtmlUnit through EventTarget. This is done by using the AT_TARGET invocation feature already supplied by EventTarget in a prior commit which is of a more robust implementation than the custom implementation in XMLHttpRequest.
Member
|
Many thanks for this great contribution. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem in brief
This PR changes the invocation order of events added to
XMLHttpRequest, through bothaddEventListener()and on-handlers, to be more consistent with major browser such as Chrome / FF / IE, as well as other events already implemented in HtmlUnit that use EventTarget's framework.Technical details
This is done by using the AT_TARGET invocation feature previously added to EventTarget, which is a more robust implementation than the custom implementation in
XMLHttpRequest.Consequently, the use of
EventTarget.executeEventLocally()to handleXMLHttpRequest's events also has this added benefits:window.currentEventis now set correctly throughout the event invocationevent.eventPhaseis set to a value consistent with major browsersNotes
I was initially worried because at a glance the
scopeparameter ofcallFunction()changes fromfunction.getParentScope()to aWindowinstance when invoked throughEventTarget.executeEventLocally(), but reading the code deeper it's not used anywhere before it's reverted to back to the samefnOrScript.getParentScope()instance ininitializeArgs()so this PR is essentially a pure refactor here.This PR also deprecates the public method
XMLHttpRequestEventTarget.getFunctionForEvent()because I could not think of a benefit of keeping itpublic. The same functionality is provided throughEventTarget.getEventHandler()which this class inherits.