[webkit-reviews] review requested: [Bug 188672] Web Inspector: Provide $event in the console when paused on an event listener : [Attachment 347302] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has asked  for review:
Bug 188672: Web Inspector: Provide $event in the console when paused on an
event listener
https://bugs.webkit.org/show_bug.cgi?id=188672

Attachment 347302: Patch

https://bugs.webkit.org/attachment.cgi?id=347302&action=review




--- Comment #6 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 347302
  --> https://bugs.webkit.org/attachment.cgi?id=347302
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347302&action=review

>>> Source/WebCore/inspector/CommandLineAPIModuleSource.js:50
>>> +	 this.$event = injectedScript._eventValue;
>> 
>> I wonder if we can just make this something like:
>> 
>>     this.__defineGetter__("$event", () => inspectedWindow.event);
>> 
>> Is $event ever different from `window.event`? Yes it is possible for the
page to have redefined the DOMWindow.prototype.event getter, so this isn't
necessarily perfect but I don't think we need to be too concerned.
> 
> I just tried this, and since `window.event` is defined, I'm not even sure
that we need this.  Typing just `event` will give you the same result as
`$event`.  I think we should hold off on this until we have a better reason for
differentiating between `$event` and `event`/`window.event`.

Considering that `window.event` is non-standard, and is more likely to be
overridden by the page's JavaScript than `$event`, I think we should consider
adding this.