1400777 - Slim down nsElementTable.h

Status: RESOLVED FIXED

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

Description

[Nicholas Nethercote [inactive]]

It has way more stuff exposed than necessary.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Remove eHTMLTags

It's just a too-cute-by-half typedef for nsHTMLTag.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - De-expose HTML group constants[]

The patch also removes some low-value comments.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - De-expose gHTMLElements[]

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 4) - Split nsHTMLElement

This patch splits out most of nsHTMLElement into a new type HTMLElement within
nsElementTable.cpp. Only the static methods IsContainer() and IsBlock() need to
remain exposed via nsHTMLElement. The patch moves TestBits() into
nsElementTable.cpp as well.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: (part 5) - Clean up nsElementTable.{cpp,h}

This patch makes some style fixes and other minor improvements.

Comment 6

[Nicholas Nethercote [inactive]]

Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f6c59b43805318812769a9177ecce9c330073c0

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8909555 [details] [diff] [review]
(part 2) - De-expose HTML group constants[]

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

::: parser/htmlparser/nsElementTable.cpp
@@ +13,5 @@
> +static const int kHTMLContent   = 0x0001; //  HEAD, (FRAMESET | BODY)
> +static const int kHeadContent   = 0x0002; //  Elements that *must* be in the head.
> +static const int kHeadMisc      = 0x0004; //  Elements that *can* be in the head.
> +
> +static const int kSpecial       = 0x0008; //  A,    IMG,  APPLET, OBJECT, FONT, BASEFONT, BR, SCRIPT, 

Nit: a couple of these lines came with trailing whitespace.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8909557 [details] [diff] [review]
(part 4) - Split nsHTMLElement

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

::: parser/htmlparser/nsElementTable.cpp
@@ +52,2 @@
>  
> +struct HTMLElement

Is it worth it to stick this in an anonymous namespace to make it "static"?

Comment 9

[Nicholas Nethercote [inactive]]

> > +struct HTMLElement
> 
> Is it worth it to stick this in an anonymous namespace to make it "static"?

What effect do anonymous namespace have on struct definitions? I tried to look this up but got inconclusive info.

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Nicholas Nethercote [:njn] from comment #9)
> What effect do anonymous namespace have on struct definitions? I tried to
> look this up but got inconclusive info.

They have the effect of not exporting the name into the global namespace (so act as "static" for the struct name) ensuring no name clashes and one symbol less for the linker to care about [1].

[1] http://en.cppreference.com/w/cpp/language/namespace#Unnamed_namespaces

Comment 11

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5835c5943a4881567e2d1e9ce1a8a6b0f97cf200
Bug 1400777 (part 1) - Remove eHTMLTags. r=mrbkap.

https://hg.mozilla.org/integration/mozilla-inbound/rev/2b23e3f158dd747de9c40c068d4e9a5da583cf98
Bug 1400777 (part 2) - De-expose HTML group constants[]. r=mrbkap.

https://hg.mozilla.org/integration/mozilla-inbound/rev/2de6882645603af89ea672180239825dce408918
Bug 1400777 (part 3) - De-expose gHTMLElements[]. r=mrbkap.

https://hg.mozilla.org/integration/mozilla-inbound/rev/50d9715e7b0ada2111baa0d51fa888135f78d1d7
Bug 1400777 (part 4) - Split nsHTMLElement. r=mrbkap.

https://hg.mozilla.org/integration/mozilla-inbound/rev/84bde7821021d9661c841e7d40bfb7e3a18393ee
Bug 1400777 (part 5) - Clean up nsElementTable.{cpp,h}. r=mrbkap.

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5835c5943a48
https://hg.mozilla.org/mozilla-central/rev/2b23e3f158dd
https://hg.mozilla.org/mozilla-central/rev/2de688264560
https://hg.mozilla.org/mozilla-central/rev/50d9715e7b0a
https://hg.mozilla.org/mozilla-central/rev/84bde7821021