[quirk.css]DD not in DL should have text-indent, not margin

RESOLVED FIXED in Future

Status

()

Core
CSS Parsing and Computation
P5
trivial
RESOLVED FIXED
19 years ago
5 years ago

People

(Reporter: tronster321, Unassigned)

Tracking

(Blocks: 1 bug, {regression, testcase})

Trunk
Future
regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [bae:20011011], URL)

Attachments

(6 attachments, 1 obsolete attachment)

(Reporter)

Description

19 years ago
For the above URL, in Netscape 4.x and IE 4 the DD tag will give the
appearance of a paragraph with the first line tabbed.  In the 1/9/99 developers
release, when viewing the page, the DD tags make the first word in each
paragraph appear tabbed but then two newlines are "added" before the next line
of text is displayed on the screen.

End Of Line.

Updated

19 years ago
Assignee: troy → rickg
Component: Layout → Parser

Comment 1

19 years ago
Rick, I think this is a DUP. It's a problem with content not nesting inside the
DD tag like it should

Updated

18 years ago
Status: NEW → ASSIGNED

Updated

18 years ago
Assignee: rickg → kipp
Status: ASSIGNED → NEW

Comment 2

18 years ago
Kipp: This is one yours. The DD tags aren't the problem. First, the <DD> is not
wrapped in a <DL>, so we don't get the "indent". I think you need to address
that. Second, if we *do* wrap the <DD> in a <DL>, then we get extra unwanted
vertical whitespace. Note that I do have another <DD> bugfix that may or may not
be checked in by the time you see this, but it's unrelated.

Here's a simple test case:

<HTML>
<BODY>
  <table width=600 border=1>
    <tr>
      <td>
        <dl>
          <dd>This</dd> page contains an archive of songs created by me, Todd
Hartley (aka Tronster).<br>
          <dd>To </dd> download the songs, click on the link to the file type
you support under the "format" column.
        </dl>
      </td>
    </tr>
  </table>
</BODY>
</HTML>

Updated

18 years ago
Status: NEW → ASSIGNED
This could probably be done in a way similar to LIs not in ULs.  See bug 1049.

Updated

18 years ago
Whiteboard: [MAKINGTEST] -- run2000@geocities.com

Comment 4

18 years ago
Created attachment 591 [details]
Examples of the coding style, with and without the DL element.

Updated

18 years ago
Whiteboard: [MAKINGTEST] -- run2000@geocities.com → [TESTCASE]

Updated

18 years ago
Assignee: kipp → rickg
Status: ASSIGNED → NEW
Summary: The DD tag has some interesting results. → {compat} The DD tag has some interesting results.

Comment 5

18 years ago
Navigators behavior of ignoring (entirely!) is a serious bug. The only real way
to handle this is in the parser - rick/harish: maybe the thing to do is when
processing a /DD tag in quirks mode is to discard it and then let your fixup
logic repair the resulting (occasional) damage.

There is really nothing I can do here in layout land.

Updated

18 years ago
Status: NEW → ASSIGNED

Updated

18 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 6

18 years ago
Fixed by improvements to elementtable.

Comment 7

18 years ago
This problem is still occurs in the latest build. The text after the </dd> should 
not break and start a new line. After talking with Rick, I will change component 
to Layout.
Status: RESOLVED → REOPENED
Component: Parser → Layout
Resolution: FIXED → ---

Comment 8

18 years ago
The content model is now correct; and layout is goofy. Reassigning to block-boy.
Assignee: rickg → buster
Status: REOPENED → NEW

Updated

18 years ago
Target Milestone: M14 → M17

Comment 9

17 years ago
This is a pretty major layout error.  Makes <DD> unusable.
Severity: minor → major
Status: NEW → ASSIGNED
Priority: P3 → P2

Comment 10

17 years ago
adding testcase kw
Keywords: testcase
Nom. nsbeta2, recc. nsbeta2+ [some lenient date-], falling through to nsbeta3 
hard stop if not fixed during nsbeta2. b.c. with html32 DD element. Per 
comments, our support for DD is currently broken. Must fix before rtm.
Keywords: nsbeta2, nsbeta3
Whiteboard: [TESTCASE]

Comment 12

17 years ago
nsbeta2+ until 6/15
Whiteboard: [nsbeta2+] [6/15]

Comment 13

17 years ago
I looked at this a little bit.  It will be very hard to fix. The problem is the 
behavior is contextual in a way that cannot be expressed in CSS.  So html.css 
needs to describe the basic behavior, and the frame constructor code will have 
to do some wampum juju to cover the other cases.  Suggest nsbeta3.

Updated

17 years ago
Whiteboard: [nsbeta2+] [6/15] → [nsbeta2-]

Comment 14

17 years ago
Cleaning up status whiteboard by marking beta2 minus (6/15 has passed)

This is also what buster (on sabbatical above) just suggested.
I think this *can* be fixed purely in CSS, although it's a bit complicated and 
should be in quirks.css only.  I'm assigning it to myself to work out the 
correct fix.
Assignee: buster → dbaron
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Summary: {compat} The DD tag has some interesting results. → [quirk.css]DD not in DL should have text-indent, not margin
Target Milestone: M17 → M18
I think this would be fixed by adding the following rules to quirks.css:

dd {
  text-indent: <some value>;
  margin-left: 0;
}

dl > dd {
  text-indent: 0;
  margin-left: 40px;
}
I think the following is the correct patch.  Do you think it's worth having the
2d and 4th added rules?

Index: quirk.css
===================================================================
RCS file: /cvsroot/mozilla/layout/html/document/src/quirk.css,v
retrieving revision 1.2
diff -u -d -r1.2 quirk.css
--- quirk.css	2000/06/11 22:48:05	1.2
+++ quirk.css	2000/07/21 18:54:16
@@ -58,6 +58,26 @@
   list-style-position: inherit;
 }
 
+/* Quirk: DD not in DL has text-indent instead of margin (bug 5119) */
+
+dd {
+  text-indent: 40px;
+  margin: 0;
+}
+
+dd > * {
+  text-indent: 0;
+}
+
+dl > dd {
+  text-indent: inherit;
+  margin: 40px;
+}
+
+dl > dd > * {
+  text-indent: inherit;
+}
+
 /* Ensure that we get proper padding if the very first node
    beneath an <li> is another <ul>. This is an ugly way to
    fix the problem, because it extends the <li> up into
Note that there are currently no other UA-level declarations of text-indent, but
if there were, the 2d and 4th rules would mess them up.
Ian - assigning this to you since it's quirk.css.  Patch that should fix it is
in the bug.
Assignee: dbaron → ianh
Status: ASSIGNED → NEW
Thanks David.

Seems like a bit of a violent fix, but since it is quirks-only, fine. Did you
test the fix with different font sizes to work out if the indent was 40 pixels
or in ems in legacy browsers?
Assignee: ianh → py8ieh=bugzilla
OS: Windows NT → All
Priority: P2 → P1
Hardware: PC → All
Whiteboard: [nsbeta2-] → [nsbeta2-] [HAVE FIX]
No, I didn't test, but apparently everything else is 40px.  Do a quick test if
you want...
Created attachment 13884 [details] [diff] [review]
patch

Comment 23

17 years ago
r=buster.  thanks for handling this one.
Will do this as part of the big ua.css changes next week.
Whiteboard: [nsbeta2-] [HAVE FIX] → [nsbeta2-] [HAVE FIX] [nsbeta3+]
Depends on: 3935
I assume that the "margin: 40px" bit should read "margin-left: 40px".

That is what I have put in my quirk.css file.
No longer depends on: 3935
Blocks: 6625

Comment 26

17 years ago
PDT thinks this is a P2 at best, pretty close to P3 since it's a longstanding
bug in all our clients.
Priority: P1 → P2
Whiteboard: [nsbeta2-] [HAVE FIX] [nsbeta3+] → [nsbeta2-] [HAVE FIX] [nsbeta3+][PDTP2]

Comment 27

17 years ago
Ian: fixed with *.css checkin, right?
Status: NEW → RESOLVED
Last Resolved: 18 years ago17 years ago
Resolution: --- → FIXED
FIXED by updating quirk.css.

Comment 29

17 years ago
Ian,

Not sure from looking at the test cases provided, how to verify this ?
Compare the two test cases with legacy browsers. They should be the same in
theory.

Comment 31

17 years ago
Netscape 4.7 and Netscape 6 both rendered the test files different as described 
in the original summary.

Netscape 6:
 First word is indented the remaining paragraph is rendered below.

Netscape 4.7:
 Entire paragraph is indented.

Comment 32

17 years ago
Reopening
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 33

17 years ago
The "valid" par of the testcase (with enclosing <dl>) is invalid; the <dd> is 
closed before the end of its content. Since this bug isn't about handling 
prematurely closed <dd>s, I'm attaching a new testcase with that part fixed.

Here's a fix for the bug itself:
In "quirk.css":

/* Quirk: DD not in DL has text-indent instead of margin (b=5119) */

-dd {
-  text-indent: 40px;
-  margin: 0;
-}
-
-dd > * {
-  text-indent: 0;
-}
-
-dl > dd {
-  text-indent: inherit;
-  margin-left: 40px;
-}
-dl > dd > * {
-  text-indent: inherit;
-}

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

Tested in Mozilla nightly build id: 2000100308

Comment 34

17 years ago
Created attachment 18765 [details]
new "examples"
That won't work, what if you have:

   <dd> one </dd>
   this is some text.
   <dd> two </dd>
   this is some text.

i.e., remove the <br>s from the testcase.
Assignee: ian → fantasai
Status: REOPENED → NEW
QA Contact: petersen → ian

Comment 36

17 years ago
Yes, I realized that later..

There's a give and take here; you can't support every broken coding. From what
I see, you either have to give up handling
  - </dd> in the middle of the visual "block" (use David Baron's "fix")
  - <dd> without a preceding line break of some kind (use the "fix" above)
  - empty <dd>s (I've got a really messy CSS hack to do this)

I'll attach a modified quirk.css demonstrating the last one.

Probably the last one gives the "safest" output. IE5 can't handle empty <dd>s 
either.

It depends on whether or not you're willing to swallow that code. =)
(I ought to attach an expanded testcase...)

Comment 37

17 years ago
Created attachment 19300 [details]
modified quirk.css

Comment 38

17 years ago
Created attachment 19352 [details]
Expanded testcase

Comment 39

17 years ago
Ran the testcase through Mozilla build 2000100308 with modified quirk.css; 
results identical to Nav4.x

I forgot to close the <h2> on the second set in the testcase. Sorry~

Comment 40

17 years ago
This bug was marked to be fixed in a previous milestone but it didn't get fixed 
properly. Nominated for beta1.
Keywords: nsbeta1

Comment 41

17 years ago
Milestone 0.8 has been released. We should either resolve this bug or update its
milestone.

Comment 42

17 years ago
fantasai, do we want to take this modified quirk.css patch? BTW: very creative ;)

Comment 43

17 years ago
Created attachment 25727 [details]
quirk.css rules
For the record: This had better stay very firmly inside quirk.css. ;-)

Comment 45

17 years ago
> fantasai, do we want to take this modified quirk.css patch? 

Nope, you'd want the rules I just attached. (I forgot to set 'font: inherit' for 
proper <dd>s)

> BTW: very creative ;)

Thanks!

Comment 46

17 years ago
Ian, can you review the patch? If you like it, I'll take it to the tree...
Well, Moz with this patch and IE display all the testcases differently, so I
guess it doesn't pass review... But it's better than what we have now, and it's
a quirk so I'm not particularly worried either way, so r=hixie.
Keywords: nsbeta2, nsbeta3
Whiteboard: [nsbeta2-] [HAVE FIX] [nsbeta3+][PDTP2]

Comment 48

17 years ago
Taking bug to checkin the patch - thanks Fantasai!
Assignee: fantasai → attinasi
Target Milestone: M18 → mozilla0.8.1

Comment 49

17 years ago
Have Patch, will checkin.
Status: NEW → ASSIGNED
Keywords: patch
I'm curious -- why all the dt:before hackery rather than just the same
margin-left on the inline dd as the block?

Comment 51

17 years ago
>thanks Fantasai!

My pleasure.

> I'm curious -- why all the dt:before hackery rather than just the same
> margin-left on the inline dd as the block?

Because the ::before psuedo-element creates a box *inside* the <dd>;
you'd get margin space before the line break instead of after it.
I tried that at first--didn't work. ^^
sr=shaver.  Can it get in the tree?

Comment 53

17 years ago
YEs - in fact, I meant to check it in over the weekedn but I was distracted by a
regression... It will go in very, very soon.

Updated

17 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED

Comment 54

17 years ago
Fantasai's change was checked in. Thanks! (quirk.css ver. 1.14)
'none' isn't a valid value for the 'content' property.  'content: none' should
be changed to 'content: ""'.

Comment 56

17 years ago
Created attachment 27995 [details]
quirk.css rules, validated
well, we're not identical to IE, but whatever... :-) VERIFIED
Status: RESOLVED → VERIFIED
Indentation of DD outside of DL no longer works.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Forgot to mention the Build ID: 2001-08-06-03 on Windows 98 SE. I have poked
around in quirk.css and found that it is this rule that is not working as it
used to:

:not(dl) > dd:before {
  display: inline;
  white-space: pre;
  font-size: 1px;
  line-height: 0;
  content: "\A  ";
  margin-right: 40px;
}

It seems that "font-size: 1px" is the cause of the problem - if I change it to
"font-size: 2px" (or higher, or remove it) it works again.
*** Bug 93980 has been marked as a duplicate of this bug. ***

Updated

16 years ago
Keywords: mozilla1.0, regression
Target Milestone: mozilla0.8.1 → ---

Comment 61

16 years ago
Are you still seeing this problem? It seems to work on 2001082803 (Win2K).
I still see this problem, 2001-09-14-03 on Windows 98 SE.
My comment on 2001-08-06 20:52 is still valid.

Comment 63

16 years ago
Why is this a valid bug? In the example that Rick gave (From rickg@netscape.com 
1999-04-26 00:14), he has pcdata within the DL -- that is illegal. The only 
content allowed within the DL container element is a DD or a DT. 

Why are we spinning our wheels for invalid html?

Updated

16 years ago
Blocks: 104166

Comment 64

16 years ago
since this deals with invalid markup, I am going to mark this bug as invalid, if 
there are issues about that, please let me know.
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago16 years ago
Resolution: --- → INVALID
Reopening.  This is a bug in our implementation of a quirk.  If you don't care
about invalid markup, then you should instead file a bug that we remove the
quirk entirely, but if we have it it ought to be right.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---

Comment 66

16 years ago
The only requirement is that the text be rendered, which it is. Is the bottom 
line issue here that we do not render it like IE? If that is the case, then this 
is P5/Futured for Marc or can be reassigned to someone else who has the cycles 
to pick it up.
Severity: major → trivial
Priority: P2 → P5
Whiteboard: [beppe:10/11/01]
Target Milestone: --- → Future
How is this bug affected by the resolution of bug 102370?

Updated

16 years ago
Whiteboard: [beppe:10/11/01] → [bae:20011011]
*** Bug 115355 has been marked as a duplicate of this bug. ***
Attachment #13884 - Attachment is obsolete: true
Keywords: patch

Comment 69

16 years ago
removing myself from the cc list

Comment 70

15 years ago
*** Bug 166558 has been marked as a duplicate of this bug. ***
FWIW, the current quirk does not work for dir=RTL pages.
(It does seem to work for LTR again though).
->style
Assignee: attinasi → dbaron
Status: REOPENED → NEW
Component: Layout → Style System

Comment 73

14 years ago
FIXED/WFM?
Keywords: mozilla1.0

Updated

13 years ago
Blocks: 56362

Comment 74

13 years ago
Again, what is the status on this bug?
Assignee: dbaron → nobody
QA Contact: ian → style-system
INVALID per HTML5. <http://www.whatwg.org/html/#margins-and-padding>
Status: NEW → RESOLVED
Last Resolved: 16 years ago7 years ago
Resolution: --- → INVALID
FIXED is a better resolution, since this bug was in fact fixed (comment 54 / comment 57), and then reopened for some other problem (comment 58 / comment 59).

But if you're citing HTML5, why aren't you filing a bug that we should remove the quirk entirely?  Would that match other browsers, or not?
Resolution: INVALID → FIXED
You need to log in before you can comment on or make changes to this bug.