Conversation
… `noPatch` mode has `composedPath`.
…rs (e.g. Chrome 41) when using unwrapped `addEventListener`.
…rget` does not exist. (IE 11)
Safari 9 both doesn't have working `Event.prototype` descriptors and doesn't allow event property getter / function call context objects to be anything other than exactly the original event, so `utils.settings.hasDescriptors` is not a sufficient basis for deciding whether or not to use the proxy-like event object wrapper.
bicknellr
changed the title
XMLHttpRequest if no EventTarget; dispatch a 'proxy' if currentTarget is an own value property descriptor
| // exists on a new `Event` and switch based off of that. Safari 9 both doesn't | ||
| // have working descriptors for `Event` properties and doesn't allow getter | ||
| // contexts to be anything other than the original event, so | ||
| // `utils.settings.hasDescriptors` is not a sufficient test for deciding this |
There was a problem hiding this comment.
Hmm, so how does this not break Safari 9? Which path does it take?
There was a problem hiding this comment.
Good catch, my comment is wrong. Safari 9 actually does have working Event descriptors, so it's OK to overwrite currentTarget on the original event. The real problem is that utils.settings.hasDescriptors is false in Safari 9 because it's set like this:
polyfills/packages/shadydom/src/utils.js
Lines 31 to 33 in 08c65f4
And Safari 9's firstChild descriptor is not configurable.
kevinpschaaf
left a comment
kevinpschaaf
left a comment
There was a problem hiding this comment.
For posterity, because this took me a bit to grok:
On Safari 9, currentTarget is not an own property of Event, but the descriptor on the prototype is also not usable (making it somewhat unique, since the unusable descriptors are all normally ownPropertys). So Safari 9 does go through the "save instance descriptor / finally restore descriptor" logic, but because the original descriptor is undefined, we just delete the shimmed one off the instance, and the prototype-but-not-good-accessor one is allowed to work again.
2 participants
In at least Chrome 41, events'
currentTargetis an own property with a broken descriptor. Setting a new descriptor on one of these events makes it impossible to restore the default behavior ofcurrentTargetfor that event, making it useless to any later listeners. Shady DOM occasionally has to manually dispatch an event, which involves temporarily setting a newcurrentTargetdescriptor to allow Shady DOM to update this property. With this PR, if thecurrentTargetproperty of events is broken as it is in Chrome 41, Shady DOM will create a new object with the event as its prototype, modify that new object'scurrentTargetdescriptor, and pass that new object to listeners during the manual dispatch.EventTargetdoesn't exist in some older browsers, so objects that would normally haveEventTarget.prototypein their prototype chain in modern browsers instead have separate copies of those properties on other prototypes.XMLHttpRequestis one of these objects withEventTarget-like properties, but these properties were unpatched until this PR.