Bug 68167 - [test case] <dt> isn't closed on </dl>
Summary: [test case] <dt> isn't closed on </dl>
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: khtml parsing (show other bugs)
Version: 4.0
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-11-14 02:25 UTC by Andy Neitzke
Modified: 2004-01-11 18:33 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
test case showing indent problem (96 bytes, text/html)
2003-11-14 02:26 UTC, Andy Neitzke
Details
improved test case which validates and shows indent problem (358 bytes, text/html)
2003-11-14 13:57 UTC, Andy Neitzke
Details
another test case which doesn't have the problem and differs only by a leading <p> (354 bytes, text/html)
2003-11-14 14:08 UTC, Andy Neitzke
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Neitzke 2003-11-14 02:25:19 UTC
Version:           4.0 (using KDE 3.1.93 (CVS >= 20031111), Mandrake Linux Cooker i586 - Cooker)
Compiler:          gcc version 3.3.1 (Mandrake Linux 9.2 3.3.1-4mdk)
OS:          Linux (i686) release 2.4.21-0.13mdk

Inside a <dl></dl> pair, the items are supposed to be indented and they are; but in some circumstances the indentation continues even after the closing </dl>.  For a "real-world" example exhibiting this problem try e.g. 

http://www.slac.stanford.edu/spires/find/hep/www?rawcmd=fin%20a%20motl
Comment 1 Andy Neitzke 2003-11-14 02:26:22 UTC
Created attachment 3214 [details]
test case showing indent problem
Comment 2 Stephan Kulow 2003-11-14 10:24:29 UTC
You're aware that this is highly invalid HTML?
Note, that all other browsers indent your test case at all. 
konqueror 3.1.4 doesn't render it like IE either, so I don't
consider it a regression, we just changed the way we handle
that broken HTML
Comment 3 Stephan Kulow 2003-11-14 10:29:16 UTC
Just in case you know the author of that page: 
validator.w3.org says
707 errors for HTML 4.01
711 errors for HTML 3.2
Comment 4 Andy Neitzke 2003-11-14 13:57:21 UTC
Created attachment 3221 [details]
improved test case which validates and shows indent problem

Oops, my fault.  Here is a better test case which validates properly and still
shows the problem.  (Note that Mozilla does indent this test case.)
Comment 5 Andy Neitzke 2003-11-14 14:08:52 UTC
Created attachment 3222 [details]
another test case which doesn't have the problem and differs only by a leading <p>

If you delete the leading <p> tag then the problem goes away.  Either way is
valid HTML according to the validator.	This behavior is similar to Bug 67916
so I guess it's possible they have a common origin.
Comment 6 Andy Neitzke 2004-01-01 04:06:31 UTC
By the way, this bug also affects some pages on developer.kde.org, which might be considered "closer to home":  see e.g.

http://developer.kde.org/documentation/library/cvs-api/kdecore/html/structKCmdLineOptions.html

Again, this page is rendered correctly by Mozilla.
Comment 7 Dirk Mueller 2004-01-11 18:33:21 UTC
Subject: kdelibs/khtml

CVS commit by mueller: 

fix handling of <p><dl>
CCMAIL: 68167-done@bugs.kde.org


  M +2 -0      ChangeLog   1.150
  M +8 -6      html/htmlparser.cpp   1.342


--- kdelibs/khtml/ChangeLog  #1.149:1.150
@@ -1,4 +1,6 @@
 2004-01-11  Dirk Mueller  <mueller@kde.org>
 
+        * html/htmlparser.cpp (insertNode): fix <p><dl> handling (#68167).
+
         * css/css_base.cpp (extractPseudoType): make it case-insensitive (#72159).
 

--- kdelibs/khtml/html/htmlparser.cpp  #1.341:1.342
@@ -478,7 +478,9 @@ bool KHTMLParser::insertNode(NodeImpl *n
         case ID_DL:
             popBlock( ID_DT );
+             if ( current->id() == ID_DL ) {
             e = new HTMLGenericElementImpl( document, ID_DD );
             insertNode( e );
             handled = true;
+             }
             break;
         case ID_DD: