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 → 24 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: 24 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: