1401097 - Simplify gHTMLElements

Status: RESOLVED FIXED

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

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Simplify gHTMLElements

mrbkap, I realize the direct changes to gHTMLElements are hard to review. It
might give you more confidence to know that (a) I did the transformation by
hand, and then (b) I did it again automatically -- iterating over the elements
and printing out the that and the results of IsBlock() and IsContainer() in the
"ELEM(...)" form -- to make sure they matched.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Remove redundant conditions in nsXHTMLContentSerializer::LineBreakAfterClose()

The removed tags don't need a separate check because nsHTMLElement::IsBlock()
will return true for them.

Comment 3

[Nicholas Nethercote [inactive]]

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

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8909622 [details] [diff] [review]
(part 1) - Simplify gHTMLElements

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

It might be worth it at some point for either Thunderbird or Editor folks to come back around to see what actually makes sense for this table... The "block level elements" were (I think) pulled from the old HTML4 DTD and then things got modified in mostly random ways to make the old parser's behavior compatible with the web. Therefore, I don't think we need to attach a lot of importance to the existing values over making our code more sane.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Oh, and this is probably the first and only code I've seen that #defines ____ to something in order to make the code *more* clear (as opposed to all of the IOCCC entries with the opposite goal) :-)

Comment 6

[Nicholas Nethercote [inactive]]

> It might be worth it at some point for either Thunderbird or Editor folks to
> come back around to see what actually makes sense for this table... The
> "block level elements" were (I think) pulled from the old HTML4 DTD and then
> things got modified in mostly random ways to make the old parser's behavior
> compatible with the web. Therefore, I don't think we need to attach a lot of
> importance to the existing values over making our code more sane.

I'm not the person to do that evaluation, but I have added a comment that (a) indicates which elements have a suspect mIsBlock value, and (b) incorporates part of your explanation, for future reference.

Comment 7

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8bde5c2445523f301edbafb037f1c10de47ac2ef
Bug 1401097 (part 1) - Simplify gHTMLElements. r=mrbkap.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e1d4f3e47fb71442ada0a8948c503634428752ad
Bug 1401097 (part 2) - Remove redundant conditions in nsXHTMLContentSerializer::LineBreakAfterClose(). r=mrbkap.

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/8bde5c244552
https://hg.mozilla.org/mozilla-central/rev/e1d4f3e47fb7