1393419 - Implement nsIContent::As<T> to replace QI(nsIDOMHTML*Element)

Status: RESOLVED INVALID

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Masatoshi Kimura [:emk]]

Attachment: Proof of concept

This is inspired by JSObject::as<T>. This is safer than a bare static_cast and has no overhead such as virtual calls.

qdot, WDYT?

Comment 1

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8900698 [details] [diff] [review]
Proof of concept

Review of attachment 8900698 [details] [diff] [review]:
-----------------------------------------------------------------

This looks ok to me. The LocalName() lookup is a little awkward but I can't find anywhere else that we have easily accessible nsGkAtoms -> binding type lookups. I may ask bz about this once he's synced back up from vacation.

::: docshell/base/nsContextMenuInfo.cpp
@@ +85,5 @@
>        content = do_QueryInterface(curr);
>        if (!content) {
>          break;
>        }
> +      if (content->IsHTMLElement(nsGkAtoms::a)) {

I think we could just move the As<>() call here instead of having to check for HTML element type first?

Comment 2

[Masatoshi Kimura [:emk]]

Attachment: Proof of concept

> I think we could just move the As<>() call here instead of having to check for HTML element type first?

Good point, fixed.

Comment 3

[Masatoshi Kimura [:emk]]

Boris, do you have better idea to get binding types from nsGkAtoms?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Well, the first problem is that you can't do it, in general.  For example, both <q> and <blockquote> are HTMLQuoteElement, so if you wanted to do As<HTMLQuoteElement> you wouldn't be able to do it using the setup with T::LocalName() or any other setup that depends on a one-to-one mapping of localnames to types.  Similar for <col> and <colgroup>, and the fact that we have an HTMLSharedElement complicates things even further.

For the cases where tags map 1-1 to element types, we already have FromContent machinery, no?

Comment 5

[Masatoshi Kimura [:emk]]

Oh, I didn't notice FromContent. Thank you.