181697 - <strong> element extends its effect unnecessarily when <LI> tag is not closed

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Katsuhiko Momoi]

** Observed with 2002-11-23 trunk build **

When you have a source like the following:

===========
...
...

<UL>
<strong>
<LI> Line 1
<LI> Line 2
</strong>
</UL>
<P>
Line 3: Outside the <strong> element.
...
...
===========

the effect of <strong> extends to Line 3. I have seen some documents
displaying mostly in bold because of this problem. 

There are a couple of ways to avoid this problem.

1. Close each of the 2 <LI> with </LI>.

or

2. Move <strong> in front of <ul>.

I'll attach a sample test case next.

Comment 1

[Katsuhiko Momoi]

Attachment: A mimimum test for showing the problem.

Comment 2

[Katsuhiko Momoi]

This is a regression from 1.0 branch.

I can see this problem in:

The latest 1.0.2 branch build.
The latest trunk build.

but not in Mozilla 1.0 or Netscape 6.2.x builds.

This is basic stuff and should be fixed as ASAP,
hopefully before the next major release.

Comment 3

[Katsuhiko Momoi]

Note that once you have this problem, all text after the
problem spot will be in bold type.

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

->Parser, not Style System.

Comment 5

[Katsuhiko Momoi]

Adding nsbeta1 to keyword.

Comment 6

[Samir Gehani]

adt: nsbeta1-

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

*** Bug 263674 has been marked as a duplicate of this bug. ***

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

This may be related to bug 256731 (auto-closing phrase-level elements seems to
be pretty broken).

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

*** Bug 207032 has been marked as a duplicate of this bug. ***

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

So in nsElementTable.cpp, I made <li> a kBlockEntity, and allowed font to
contain it through gFontKids... but this changes the regression test
bug7823.html since we used to allow <li> as a child of <b>, and with my change
we don't. So because <b> is a kFontStyle, it was auto-closing the <li> since it
could contain it <li> (see
http://lxr.mozilla.org/seamonkey/source/parser/htmlparser/src/nsElementTable.cpp#2166
for the gory details) and now we don't close it since kFontStyles cannot contain
kBlockEntities.

Note that with my change, we parse that testcase much closer to IE (it does
funky things with the DOM that we don't do, since we do RS and it just seems to
deal with mismatched close tags).

Also, the parser regression tests exploded when I tried to run them, so I think
something screwy is going on there, too, but I doubt it's related to this bug.

I'll attach a patch tomorrow at some point. bz, rbs, do you have any opinions here?

Comment 11

[Boris Zbarsky [:bzbarsky]]

> but this changes the regression test bug7823.html

Reading bug 7823, it sounds like it actually refixes that bug (which got broken
by the patch that made li a flow element...)

Parsing closer to IE is a good thing anyway.  ;)

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v1

This is the described patch. One style note: this file makes no attempt to
respect the 80 char limit, and I don't think now is the time to start enforcing
such a limit.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 163891 [details] [diff] [review]
patch v1

>Index: src/nsElementTable.cpp
>+      /*parent,incl,exclgroups*/          kBlockEntity, kFlowEntity, kSelf, // changed this to kBlockEntity so we enable RS handling for 

s/this to kBlockEntity/this back to kBlockEntity/ and r=bzbarsky

Comment 14

[rbs]

Comment on attachment 163891 [details] [diff] [review]
patch v1

sr=rbs

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked in.

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

*** Bug 139689 has been marked as a duplicate of this bug. ***