8749 - [PATCH] nested <DL> tags don't display properly

Status: VERIFIED FIXED

(Core :: Layout, defect, P2)

Description

[Luis Villa]

Clarifications:

*I'm using the M7 build, despite the "other" tag above.
*The given URL does not display the error; however, after going to that URL,
clicking on "read the boards" and then any "read the thread" button will
reproduce the problem. I've attached a code snippet to demonstrate the usage in
the page.

Description:

The page uses nested <DL> tags within a post form to provide indentation for a
threaded BBS (use of older Netscapes, IE4, and Lynx all generate the desired,
tree-like, indentation.) Mozilla only recognizes the first <DL>, which leaves
lower-level posts un-indented, resulting in a flat structure without the desired
indentation.

in other words, a page should appear as so:
<dl>
	<dl>
		<dl>
		</dl>
	</dl>
</dl>

but is in fact rendered as:
<dl>
<dl>
<dl>
</dl>
</dl>
</dl>


AFAICT, HTML 4.0 allows this nested usage of the DL tag, though it is
"discouraged." The standard says that exact presentation depends on the user
agent, so although this presentation is probably technically
standards-compliant, it is probably not as correct as it could be.

I've also checked, and the <dl *compact*> in the HTML below is not the cause of
the problem.

HTML:
<p><form method=post action=http://www.sagarmatha.com/dbrboard/bin/bplist.cgi>
<dl compact>
<dd>
<input type=radio checked name=article
value=/home/james/sagarmatha/dbrboard/boards/3830/message.txt>&nbsp;&nbsp;&nbsp;<font
color=#00ff00>Report from Nike Camp (1989)
 - Bubba
 (06/22/99 18:53)</font></dd>
<dl compact>
<dd>
<input type=radio  name=article
value=/home/james/sagarmatha/dbrboard/boards/3830/1/message.txt>&nbsp;&nbsp;&nbsp;<font
color=#ff8800><b>Great fun, looking forward to next (dated) report **
 - Jason Evans
 (06/23/99 06:53)</b></font></dd>
<dl compact>
</dl>
</dl>
</dl>
<center><table><tr><td align=center><input type=submit name=submit value="Read
the Article">
</td></tr></table></form>

Comment 1

[rickg]

Chris -- can you verify this on linux and mac? It works perfectly (or I'm
missing the problem) on NT.

Comment 2

[Chris Petersen]

With the latest build (1999062808), I can reproduce the problem described with
nested DL elements. This problem occurs on all platforms. I have attached a
simply case that reproduces the problem.

Comment 3

[Chris Petersen]

Attachment: This test case contains a nested set of DL's

Comment 4

[rickg]

Kipp - It's a low priority -- but this one is a layout problem. The content
model is formed exactly as you'd expect.

Comment 5

[kipp]

I updated the ua.css so that nested DL's indent properly

Comment 6

[Chris Petersen]

Fixed in the July 16th Build (All Platforms)

Comment 7

[Luis Villa]

Re-opening. I assume the source of the bug is not the same, but the rendering is
broken again in basically exactly the same way as it was two years ago. The old
testcase still demonstrates the problem just fine. This was fine with the build
from two days ago (I still have that tarball; I can check the exact stamp on
that build in a couple minutes) but is broken in 2001031405. 

Comment 8

[Luis Villa]

For reference, this worked with 2001031121. Additionally, the bug is so old the
parent URL has changed. Updating that with a more useful one.

Comment 9

[Luis Villa]

Forgot to add the keyword.

Comment 10

[Mike Shaver (:shaver emeritus)]

Reassigning to default component owners.

Comment 11

[karnaze (gone)]

Reassigning to attinasi and moving to m1.0.1

Comment 12

[Luis Villa]

Some more details: 2001031208 works, 2001031221 does not, and the builds in
between segfault for me. So the brokenness was checked in some time during the
work day on 03/12. Hope that helps track down the bug.

Comment 13

[Marc Attinasi]

Looks like this rule has to be added to quirks.css (must have gotten removed?)

dl > dd, dl > dl {
  display: block;
  margin-left: 40px;
}


note: now it is in as:

dl > dd {
  display: block;
  margin-left: 40px;
}

Comment 14

[Luis Villa]

Solves the problem for me. Thanks!

Comment 15

[Luis Villa]

Um... so is this fix ever going to get checked in? It's really irritating to
have to re-edit this every day when I get a new build, and every other major
browser except IE 5 for Mac handles this page correctly. It isn't a major
correctness issue, but it /is/ a correctness/NS4.7 issue and should be fixed. 

Comment 16

[Marc Attinasi]

I'll check it in if I can get a review and super-review.

liv - do you want to review the change? You have been running with it.

BTW: sorry to have caused you annoyance, but this is a pretty esoteric case so
it has not gotten much attention from me.

Comment 17

[Luis Villa]

Sorry to give the impression I was terribly annoyed. Obviously, this is a minor
case and you guys all have lots of things on your plates. I just didn't want it
to slip through the cracks :) 
Anyway, I'm not sure that I have what it takes to review, but I have been
running this fix for a month so I guess I can throw in an 

r=[e-mail]

for what it is worth. 

Comment 18

[Marc Attinasi]

Patch for approval:

Index: mozilla/layout/html/document/src/quirk.css
===================================================================
RCS file: /cvsroot/mozilla/layout/html/document/src/quirk.css,v
retrieving revision 1.16
diff -u -3 -r1.16 quirk.css
--- quirk.css	2001/03/24 06:25:33	1.16
+++ quirk.css	2001/05/15 23:21:16
@@ -186,7 +186,8 @@
   content: "";
   margin: 0;
 }
-dl > dd {
+/* dl > dl quirk to allow indenting of nested dl  (b=8749) */
+dl > dd, dl > dl {
   display: block;
   margin-left: 40px;
 }

Comment 19

[Luis Villa]

I don't mean to be a whiny bitch, but _every_single_other_browser_on_the_planet_
(IE, Netscape 4.x, Opera, and the new OS X one whose name escapes me at the
moment) renders this tag sequence correctly. And it is a one line patch! How
hard can it be to get this in to the tree and close this out? 

Comment 20

[Marc Attinasi]

Attachment: Patch updated since dbaron;s changes to dl / dd handling

Comment 21

[Marc Attinasi]

OK liv, I'll get this in to the trunk. Sorry to drive you to whining...

Comment 22

[Chris Waterson]

Why is this quirks-mode only?

Comment 23

[Luis Villa]

Chris: I think that the original assessment (~M7) was that it was a quirk
because the HTML spec says that this doesn't /have/ to be supported, and that
CSS should be used instead. Of course, this is untenable if you want to support
Lynx, NS 3/4, etc., so even though the spec says "this isn't the right way"
every browser on the planet does it anyway. So... it is permissible under the
spec but not required, hence quirks. 

Comment 24

[Chris Waterson]

Ok, dbaron just enlightened me as to dl's not containing dl's. So quirks-only is
cool, sr=waterson.

Comment 25

[Marc Attinasi]

Checked into trunk (quirk.css ver 1.25)

Comment 26

[Marc Attinasi]

Marking FIXED, but if anybody thinks there is a case for including this in the
0.9.2 branch, please reopen and state your case...

Comment 27

[Luis Villa]

Dumb question: I'm not following as closely as I used to :/ if it doesn't go in
for 0.9.2, when /will/ it go in? Still for 1.0 or for post 1.0?

Comment 28

[Marc Attinasi]

Since it is in the trunk now, it will be in all subsequent Mozilla releases,
certainly 1.0, or earlier milestones if there are any (unless someone backs it
out, of course).

Comment 29

[Marc Attinasi]

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

Comment 30

[Chris Petersen]

Marking verified in the Sept 06 build (2001-09-06-05)