Instead of the optional `element` parameter, the View constructor should take an `elementOrIdentifier` which can be an HTMLElement or id attribute value.
<rdar://problem/28365848>
Created attachment 289236 [details] Patch
Comment on attachment 289236 [details] Patch Clearing flags on attachment: 289236 Committed r206101: <http://trac.webkit.org/changeset/206101>
All reviewed patches have been landed. Closing bug.
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?
(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.
(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?".