Bug 174719 - [WebIDL] Make a few parameters non-nullable in inspector IDL files
Summary: [WebIDL] Make a few parameters non-nullable in inspector IDL files
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-21 11:01 PDT by Sam Weinig
Modified: 2017-07-21 12:40 PDT (History)
3 users (show)

See Also:


Attachments
Patch (9.83 KB, patch)
2017-07-21 11:02 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2017-07-21 11:01:33 PDT
[WebIDL] Make a few parameters non-nullable in inspector IDL files
Comment 1 Sam Weinig 2017-07-21 11:02:20 PDT
Created attachment 316105 [details]
Patch
Comment 2 Joseph Pecoraro 2017-07-21 11:22:53 PDT
Comment on attachment 316105 [details]
Patch

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

r=me

> Source/WebCore/inspector/InspectorFrontendHost.cpp:392
>      if (!is<MouseEvent>(event))
>          return;

If this is the case should we just change the IDL to take a MouseEvent instead of an Event?
Comment 3 Joseph Pecoraro 2017-07-21 11:41:39 PDT
`InspectorFrontendHost.dispatchEventAsContextMenuEvent` can take a non-nullable. It is only used in our code with a non-null object:

>   if (this._event.type !== "contextmenu" && typeof InspectorFrontendHost.dispatchEventAsContextMenuEvent === "function") {
>       this._menuObject = menuObject;
>       this._event.target.addEventListener("contextmenu", this, true);
>       InspectorFrontendHost.dispatchEventAsContextMenuEvent(this._event);
>   }


`CommandLineAPIHost.getEventListeners` taking a non-nullable would be a small behavior change but probably for the better.

Users can use this by just calling it from Web Inspector's Quick Console:

    js> getEventListeners()
    undefined

    js> getEventListeners(document.body)
    {}

    js> getEventListeners(node)
    {click: Array}

I think making the first argument non-nullable would change that first case to:

    > getEventListeners()
    TypeError: Not enough arguments

Which is totally fine to me.

As something only available through Web Inspector, we wouldn't be breaking any existing code.

That said, returning `undefined` does matches Chrome's impl. None of this is standard nor commonly used. The doc everyone uses is:
https://getfirebug.com/wiki/index.php/Command_Line_API#getEventListeners.28.29
Comment 4 WebKit Commit Bot 2017-07-21 12:40:00 PDT
Comment on attachment 316105 [details]
Patch

Clearing flags on attachment: 316105

Committed r219735: <http://trac.webkit.org/changeset/219735>
Comment 5 WebKit Commit Bot 2017-07-21 12:40:02 PDT
All reviewed patches have been landed.  Closing bug.