Bug 162165 – Web Inspector: Make it easier to create a view from an existing DOM element

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Matt Baker]

Instead of the optional `element` parameter, the View constructor should take an `elementOrIdentifier` which can be an HTMLElement or id attribute value.

[Radar WebKit Bug Importer]

<rdar://problem/28365848>

[Matt Baker]

Created attachment 289236 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 289236 [details]
Patch

Clearing flags on attachment: 289236

Committed r206101: <http://trac.webkit.org/changeset/206101>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

Comment on attachment 289236 [details]
Patch

This doesn't feel like an improvement to me. Previously we were guaranteed a single type element || null. Now it can be two different types with even less clarity at the call site. Was there a particular driver for this change?

[Matt Baker]

(In reply to comment #5)
> Comment on attachment 289236 [details]
> Patch
> 
> This doesn't feel like an improvement to me. Previously we were guaranteed a
> single type element || null. Now it can be two different types with even
> less clarity at the call site. Was there a particular driver for this change?

It was done as a drive-by simplification why working on another patch. The parameter overloading idiom is used elsewhere, but I'm fine with rolling back if you think its less clear.

[Joseph Pecoraro]

(In reply to comment #6)
> (In reply to comment #5)
> > Comment on attachment 289236 [details]
> > Patch
> > 
> > This doesn't feel like an improvement to me. Previously we were guaranteed a
> > single type element || null. Now it can be two different types with even
> > less clarity at the call site. Was there a particular driver for this change?
> 
> It was done as a drive-by simplification why working on another patch. The
> parameter overloading idiom is used elsewhere, but I'm fine with rolling
> back if you think its less clear.

Yeah, I don't see this as a simplification. We should optimize for making the call site as clear as possible. And this makes it more difficult to read. Specifically, I see this and go "is that an identifier, or a class name?".