Closed Bug 5119 Opened 25 years ago Closed 14 years ago

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

Categories

(Core :: CSS Parsing and Computation, defect, P5)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Future

People

(Reporter: tronster321, Unassigned)

References

(
URL
)

Details

(Keywords: regression, testcase, Whiteboard: [bae:20011011])

Attachments

(6 files, 1 obsolete file)

Examples of the coding style, with and without the DL element.
1.13 KB, text/html
Details
new "examples"
1.18 KB, text/html
Details
modified quirk.css
4.19 KB, text/css
Details
Expanded testcase
4.16 KB, text/html
Details
quirk.css rules
460 bytes, text/plain
Details
quirk.css rules, validated
458 bytes, text/plain
Details 
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.
Assignee: troy → rickg
Component: Layout → Parser
Rick, I think this is a DUP. It's a problem with content not nesting inside the
DD tag like it should
Status: NEW → ASSIGNED
Assignee: rickg → kipp
Status: ASSIGNED → NEW
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>
Status: NEW → ASSIGNED
This could probably be done in a way similar to LIs not in ULs.  See bug 1049.
Whiteboard: [MAKINGTEST] -- run2000@geocities.com
Whiteboard: [MAKINGTEST] -- run2000@geocities.com → [TESTCASE]
Assignee: kipp → rickg
Status: ASSIGNED → NEW
Summary: The DD tag has some interesting results. → {compat} The DD tag has some interesting results.
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.
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Fixed by improvements to elementtable.
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 → ---
The content model is now correct; and layout is goofy. Reassigning to block-boy.
Assignee: rickg → buster
Status: REOPENED → NEW
Target Milestone: M14 → M17
This is a pretty major layout error.  Makes <DD> unusable.
Severity: minor → major
Status: NEW → ASSIGNED
Priority: P3 → P2
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]
nsbeta2+ until 6/15
Whiteboard: [nsbeta2+] [6/15]
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.
Whiteboard: [nsbeta2+] [6/15] → [nsbeta2-]
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...
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
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]
Ian: fixed with *.css checkin, right?
Status: NEW → RESOLVED
Closed: 25 years ago → 24 years ago
Resolution: --- → FIXED
FIXED by updating quirk.css.
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.
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.

Reopening
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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
Attached file 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

    
    

    
  
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...)

    
    

    
  

      
      
      
      

        Attached file
          
        modified quirk.css
        — 
      

    


  

    
    

    
  

      
      
      
      

        Attached file
          
        Expanded testcase
        — 
      

    


  

    
    

    
  
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~

    
    

    
  
This bug was marked to be fixed in a previous milestone but it didn't get fixed 
properly. Nominated for beta1.

Keywords: nsbeta1

    
    

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

    
    

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

    
    

    
  

      
      
      
      

        Attached file
          
        quirk.css rules
        — 
      

    


  

    
    

    
  
For the record: This had better stay very firmly inside quirk.css. ;-)

    
    

    
  
> 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!

    
    

    
  
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]

    
    

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

    
    

    
  
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?

    
    

    
  
>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?


    
    

    
  
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.

    
  
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 23 years ago
Resolution: --- → FIXED

    
    

    
  
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: ""'.

    
    

    
  


  

    
    

    
  
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. ***
Keywords: mozilla1.0, 
          
            regression
Target Milestone: mozilla0.8.1 → ---

    
    

    
  
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.

    
    

    
  
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?

    
  
Blocks: 104166

    
    

    
  
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
Closed: 23 years ago → 23 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 → ---

    
    

    
  
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?

    
  
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

    
    

    
  
removing myself from the cc list

    
    

    
  
*** 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

    
    

    
  
FIXED/WFM?
Keywords: mozilla1.0

    
  
Blocks: 56362

    
    

    
  
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
Closed: 23 years ago → 14 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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: