Closed Bug 1401097 Opened 4 years ago Closed 4 years ago

Simplify gHTMLElements

Categories

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

56 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: n.nethercote, Assigned: n.nethercote)

References

Details

Attachments

(2 files)

No description provided.
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.
Attachment #8909622 - Flags: review?(mrbkap)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
The removed tags don't need a separate check because nsHTMLElement::IsBlock()
will return true for them.
Attachment #8909623 - Flags: review?(mrbkap)
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.
Attachment #8909622 - Flags: review?(mrbkap) → review+
Attachment #8909623 - Flags: review?(mrbkap) → review+
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) :-)
Priority: -- → P3
> 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.
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.
https://hg.mozilla.org/mozilla-central/rev/8bde5c244552
https://hg.mozilla.org/mozilla-central/rev/e1d4f3e47fb7
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Duplicate of this bug: 1395836
You need to log in before you can comment on or make changes to this bug.