This setup doesn't really work for the WebIDL bindings. Instead, we should have everyone forward the XPIDL methods to nsGenericHTMLElement, which calls the WebIDL methods, and have subclasses override the WebIDL methods where necessary. Patches are more or less loosely based on Peter's queue.

I went for a virtual TabIndexDefault() instead of overriding TabIndex() to avoid duplicating code too much.

Comment on attachment 666217 [details] [diff] [review] Part c: draggable Review of attachment 666217 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/html/content/src/nsHTMLAnchorElement.cpp @@ +183,4 @@ > } > > // no href, so just use the same behavior as other elements > + return nsGenericHTMLElement::Draggable(); FWIW, I would have done: if (!HasAttr(kNameSpaceID_None, nsGkAtoms::href)) { return nsGenericHTMLElement::Draggable(); } return !AttrValueIs(kNameSpaceID_None, nsGkAtoms::draggable, nsGkAtoms::_false, eIgnoreCase);

Comment on attachment 666218 [details] [diff] [review] Part d: click() Review of attachment 666218 [details] [diff] [review]: ----------------------------------------------------------------- r=me with s/MozClick/DOMClick/g ::: dom/interfaces/html/nsIDOMHTMLElement.idl @@ +48,5 @@ > * > * See <http://www.whatwg.org/html5/#the-hidden-attribute>. > */ > attribute boolean hidden; > + [binaryname(MozClick)] I would prefer naming this DOMClick(). MozClick() gives the impression that it's a mozilla extensions while DOMClick() makes it clear it's the DOM API click() method IMO.

Comment on attachment 666219 [details] [diff] [review] Part e: focus() Review of attachment 666219 [details] [diff] [review]: ----------------------------------------------------------------- r=me with s/MozFocus/DOMFocus/g and other comments applied. ::: content/html/content/src/nsGenericHTMLElement.cpp @@ +202,5 @@ > if (!fm->GetFocusedContent() || > fm->GetFocusedContent()->OwnerDoc() != document) { > + mozilla::ErrorResult rv; > + mElement->Focus(rv); > + return rv.ErrorCode(); Can't you do: return MozFocus(); ::: content/html/content/src/nsHTMLLegendElement.cpp @@ +172,5 @@ > bool aIsTrustedEvent) > { > // just use the same behaviour as the focus method > + mozilla::ErrorResult rv; > + Focus(rv); I would prefer: Focus(mozilla::ErrorResult());

Comment on attachment 666220 [details] [diff] [review] Part f: innerHTML Review of attachment 666220 [details] [diff] [review]: ----------------------------------------------------------------- r=me with s/MozInnerHTML/DOMInnerHTML/g ::: content/html/content/src/nsGenericHTMLElement.cpp @@ +1345,5 @@ > } > mb.RemovalDone(); > > nsAutoScriptLoaderDisabler sld(doc); > As long as you are here, could you remove those two whitespaces? ::: content/html/content/src/nsHTMLStyleElement.cpp @@ +270,3 @@ > > SetEnableUpdates(true); > As long as you are here, could you remove the two whitespaces on the lines before and after SetEnableUpdates(true);

Just to make absolutely sure that we're not getting slower here.

https://hg.mozilla.org/mozilla-central/rev/78978886622b https://hg.mozilla.org/mozilla-central/rev/80ea49628d76 https://hg.mozilla.org/mozilla-central/rev/f5012893e985 https://hg.mozilla.org/mozilla-central/rev/26167a82736d https://hg.mozilla.org/mozilla-central/rev/9f999446be73 https://hg.mozilla.org/mozilla-central/rev/9d42fde0e996