Bug 146165 - Konqueror not applying CSS to MENU elements correctly
Summary: Konqueror not applying CSS to MENU elements correctly
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Debian testing Linux
: NOR normal
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
: 155279 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-05-30 18:21 UTC by Ambrose Li
Modified: 2008-01-08 18:21 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Example of the problem (1.12 KB, text/html)
2007-05-30 19:11 UTC, Ambrose Li
Details
Test case with menu tags (1.14 KB, text/html)
2007-05-31 09:06 UTC, Tommi Tervo
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ambrose Li 2007-05-30 18:21:09 UTC
Version:           3.5.3 (using KDE KDE 3.5.5)
Installed from:    Debian testing/unstable Packages
OS:                Linux

Given identical HTML and identical CSS, except for the difference that one uses the MENU tag and the other uses the UL tag, Konqueror will render the one that uses MENU in a very inconsistent way.                                                                                               
                                                                                                                                   
The following HTML snippet illustrates the problem:                                                                                 
                                                                                                                                    
----------- cut here -------------- 8< -----------------------------                                                                
<style type="text/css">                                                                                                             
                                                                                                                                    
html, body {                                                                                                                        
   color: black;                                                                                                                    
   background-color: white;                                                                                                         
}                                                                                                                                   
                                                                                                                                    
/* All links */                                                                                                                     
#topnav a {                                                                                                                         
   text-decoration: none;                                                                                                           
   color: inherit;                                                                                                                  
   padding-left: 0.5em;                                                                                                             
   padding-right: 0.5em;
}                                                                                                                                   
#topnav a:hover {                                                                                                                   
   color: white;                                                                                                                    
   background-color: black;                                                                                                         
}                                                                                                                                   
#topnav ul {                                                                                                                        
   list-style: none;                                                                                                                
}                                                                                                                                   
                                                                                                                                    
/* Main horizontal menu */                                                                                                          
#topnav ul li {                                                                                                                     
   float: left;                                                                                                                     
}                                                                                                                                   
                                                                                                                                    
/* Submenus */                                                                                                                      
#topnav li > ul {                                                                                                                   
   display: none;                                                                                                                   
}                                                                                                                                   
#topnav li:hover > ul {                                                                                                             
   display: block;                                                                                                                  
}                                                                                                                                   
#topnav li ul {                                                                                                                     
   position: absolute;                                                                                                              
   float: none;                                                                                                                     
   padding-left: 0;
   border: 2px solid black;                                                                                                         
}                                                                                                                                   
#topnav li ul li {                                                                                                                  
   display: block;                                                                                                                  
   position: relative;                                                                                                              
   float: none;                                                                                                                     
}                                                                                                                                   
#topnav li ul li a {                                                                                                                
   display: block;                                                                                                                  
}                                                                                                                                   
                                                                                                                                    
</style>                                                                                                                            
                                                                                                                                    
<div id=topnav>                                                                                                                     
<ul>                                                                                                                                
<li><a href="#">File</a>                                                                                                            
    <ul>                                                                                                                            
    <li><a href="#">New</a>                                                                                                         
    <li><a href="#">Open</a>                                                                                                        
    <li><a href="#">Close</a>                                                                                                       
    <li><a href="#">Save</a>                                                                                                        
    <li><a href="#">Quit</a>                                                                                                        
    </ul>                                                                                                                           
<li><a href="#">Edit</a>                                                                                                            
    <ul>
    <li><a href="#">Undo</a>                                                                                                        
    <li><a href="#">Cut</a>                                                                                                         
    <li><a href="#">Copy</a>                                                                                                        
    <li><a href="#">Paste</a>                                                                                                       
    </ul>                                                                                                                           
<li><a href="#">View</a>                                                                                                            
<li><a href="#">Help</a>                                                                                                            
</ul>                                                                                                                               
</div>                                                                                                                              
----------- cut here -------------- 8< -----------------------------                                                                
                                                                                                                                    
This will generate a screen that looks like a normal GUI with File,                                                                 
Edit, View and Help menus. If the user hovers the cursor over File                                                                  
or Edit, a submenu is displayed; i.e., the screen looks like this:                                                                  
                                                                                                                                    
      File Edit View Help                                                                                                           
                                                                                                                                    
and, when hovered over File,                                                                                                        
                                                                                                                                    
      File Edit View Help                                                                                                           
      New                                                                                                                           
      Open                                                                                                                          
      Close                                                                                                                         
      Save                                                                                                                          
      Quit
                                                                                                                                    
and, when hovered over Edit,                                                                                                        
                                                                                                                                    
      File Edit View Help                                                                                                           
           Undo                                                                                                                     
           Cut                                                                                                                      
           Copy                                                                                                                     
           Paste                                                                                                                    
                                                                                                                                    
This is also the way Mozilla displays the HTML.                                                                                     
                                                                                                                                    
If the "ul" tags are replaced with "menu" tags, the screen will                                                                     
look completely garbled like this:                                                                                                  
                                                                                                                                    
      File New Open Close Save Quit * Edit                                                                                          
   * View                                                                                                                           
   * Help                                                                                                                           
                                                                                                                                    
The visible differences between this and how Konqueror renders the                                                                  
"ul" version are:                                                                                                                   
                                                                                                                                    
- The first submenu are interspersed between File and Edit                                                                          
                                                                                                                                    
- Unexpected bullets and alignment for the last 3 items                                                                             

The inconsistencies seem to be caused by Konqueror being confused                                                                   
by nested MENU elements, as can be seen from the following symptoms:                                                                
                                                                                                                                    
- The text contained inside the LI elements inside the nested MENU                                                                  
  are displayed as if they were part of the outermost MENU                                                                          
                                                                                                                                    
- The close tag of the nested MENU caused Konqueror to think that                                                                   
  the outermost MENU has closed                                                                                                     
                                                                                                                                    
- Subsequent LI elements seem to have caused Konqueror to insert                                                                    
  an implicit UL, thus causing the styling for MENU (specifically                                                                   
  the list-style and float properties) to be not applied                                                                            
                                                                                                                                    
- The Edit MENU is nested within the implicit UL (if UL is indeed                                                                   
  the implicitly-inserted tag), and Konqueror is thus not confused                                                                  
  by the nesting, and displays the submenu as expected                                                                              
                                                                                                                                    
The behaviour of MENU should be identical to UL. So nested MENU's                                                                   
are definitely allowed.

---
Note for Debian people: MENU is a valid element in 4.01, and the test case is rendered correctly by both Firefox and Safari.
Comment 1 Maksim Orlovich 2007-05-30 18:39:56 UTC
Could you please attach your example as a fire? Bugzilla seems to have messed it up
Comment 2 Ambrose Li 2007-05-30 19:11:48 UTC
Created attachment 20731 [details]
Example of the problem
Comment 3 Ambrose Li 2007-05-30 19:14:23 UTC
My example is attached. It should work correctly on Konqueror as-is, but if we replace all UL tags with MENU tags, then it would stop working. 
Comment 4 Tommi Tervo 2007-05-31 09:06:24 UTC
Created attachment 20739 [details]
Test case with menu tags
Comment 5 Germain Garand 2007-05-31 10:01:28 UTC
mmh, MENU and DIR are complete legacy elements and should be treated exactly as UL. It's not the case presently.
Comment 6 Maksim Orlovich 2007-05-31 17:25:07 UTC
There are at least couple factors in that... Here are some diffs in their handling at least:

1) dtd.cpp treats them a bit differently --- that actually matches the official DTD(!) but I suspect that's in error... but doing the below doesn't seem to make a diff (I really need that DOM tree viewer built!)

--- html/dtd.cpp        (revision 669932)
+++ html/dtd.cpp        (working copy)
@@ -674,13 +674,11 @@
         return check_array(childID, tag_list_6) || check_array(childID, tag_list_1);
     case ID_OL:
     case ID_UL:
+    case ID_DIR:
+    case ID_MENU:
         // OL: LI +
+       // For DIR and MENU, the DTD says - %block, but it contradicts spec language..
         return check_array(childID, tag_list_1);
-    case ID_DIR:
-    case ID_MENU:
-        // (DIR|MENU): LI + - _3
-        if(childID == ID_LI) return true;
-        return false;
     case ID_FORM:
         // FORM: _1 * - FORM
         return check_array(childID, tag_list_1);

.. Oh, woops, there is also additional stuff in add/removeForbidden. WIll try with it

2) the default style sheet doesn't quite handle them the same...
Comment 7 Maksim Orlovich 2007-05-31 18:02:02 UTC
OK, so if I change the forbidden tags stuff as well, it mostly works -- but there is a huge space between the menu and menubar.. I presume it's due to the default CSS?
Comment 8 Germain Garand 2007-06-01 02:18:53 UTC
> I presume it's due to the default CSS? 

I'd think so, yes... we have a complex set of rules for nested ul that suppress the top/bottom margins. Those don't take menu and dir into account, so it might be just that.

Your changes look fine otherwise but I see a couple occurences of direct ID_UL usage in rendering/render_{list,object}.cpp where ID_MENU/ID_DIR could be added for sake of completness.
Comment 9 Germain Garand 2007-06-16 00:16:27 UTC
if you persist in not applying that patch, I will :)
Comment 10 Maksim Orlovich 2007-06-16 00:17:55 UTC
Feel free, my patches-to-apply buffer is overflowing ...
Comment 11 Germain Garand 2007-07-23 02:26:35 UTC
SVN commit 691136 by ggarand:

fix MENU/DIR tags handling and default css styling.
patch by Maksim Orlovich and me.

BUG: 146165



 M  +25 -10    css/html4.css  
 M  +3 -63     html/dtd.cpp  
 M  +2 -1      rendering/render_list.cpp  


--- trunk/KDE/kdelibs/khtml/css/html4.css #691135:691136
@@ -258,16 +258,30 @@
         -khtml-flow-mode: -khtml-around-floats;
 }
 
-
-ul ul, ol ul {
-	list-style-type: circle;
+ul   ul ul,   ul   ol ul,     ul   menu ul,   ul   dir ul, 
+ol   ul ul,   ol   ol ul,     ol   menu ul,   ol   dir ul,
+menu ul ul,   menu ol ul,     menu menu ul,   menu dir ul,
+dir  ul ul,   dir  ol ul,     dir  menu ul,   dir  dir ul,
+ul   ul menu, ul   ol menu,   ul   menu menu, ul   dir menu, 
+ol   ul menu, ol   ol menu,   ol   menu menu, ol   dir menu,
+menu ul menu, menu ol menu,   menu menu menu, menu dir menu,
+dir  ul menu, dir  ol menu,   dir  menu menu, dir  dir menu,
+ul   ul dir,  ul   ol dir,    ul   menu dir,  ul   dir dir, 
+ol   ul dir,  ol   ol dir,    ol   menu dir,  ol   dir dir,
+menu ul dir,  menu ol dir,    menu menu dir,  menu dir dir,
+dir  ul dir,  dir  ol dir,    dir  menu dir,  dir  dir dir
+{
+  list-style-type: square;
 }
 
-ol ol ul, ol ul ul, ul ol ul, ul ul ul {
-	list-style-type: square;
+ul   ul, ul   ol, ul   menu, ul   dir,
+ol   ul, ol   ol, ol   menu, ol   dir,
+menu ul, menu ol, menu menu, menu dir,
+dir  ul, dir  ol, dir  menu, dir  dir
+{
+  list-style-type: circle;
 }
 
-
 dd {
 	display: block;
 }
@@ -289,10 +303,11 @@
 	display: compact;
 }
 
-ol ul,
-ul ol,
-ul ul,
-ol ol {
+ol   ul, ol   menu, ol   dir, ol   ol,
+ul   ul, ul   menu, ul   dir, ul   ol,
+menu ul, menu menu, menu dir, menu ol,
+dir  ul, dir  menu, dir  dir, dir  ol
+{
 	margin-top: auto;
 	margin-bottom: auto;
 }
--- trunk/KDE/kdelibs/khtml/html/dtd.cpp #691135:691136
@@ -552,14 +552,12 @@
                (!strict && check_flow(childID, true));
     case ID_OL:
     case ID_UL:
+    case ID_DIR:
+    case ID_MENU:
         // OL: LI +
+        // For DIR and MENU, the DTD says - %block, but it contradicts spec language.. 
         return (childID == ID_LI || childID == ID_TEXT) || 
                (!strict && check_flow(childID, true));
-    case ID_DIR:
-    case ID_MENU:
-        // (DIR|MENU): LI + - _3
-        if(childID == ID_LI) return true;
-        return false;
     case ID_FORM:
         // FORM: %flow * - FORM
         return check_flow(childID, strict);
@@ -674,35 +672,6 @@
         //forbiddenTags[ID_SUP]++;
         forbiddenTags[ID_BASEFONT]++;
         break;
-    case ID_DIR:
-    case ID_MENU:
-        forbiddenTags[ID_P]++;
-        forbiddenTags[ID_H1]++;
-        forbiddenTags[ID_H2]++;
-        forbiddenTags[ID_H3]++;
-        forbiddenTags[ID_H4]++;
-        forbiddenTags[ID_H5]++;
-        forbiddenTags[ID_H6]++;
-        forbiddenTags[ID_UL]++;
-        forbiddenTags[ID_OL]++;
-        forbiddenTags[ID_DIR]++;
-        forbiddenTags[ID_MENU]++;
-        forbiddenTags[ID_PRE]++;
-        forbiddenTags[ID_PLAINTEXT]++;
-        forbiddenTags[ID_XMP]++;
-        forbiddenTags[ID_DL]++;
-        forbiddenTags[ID_DIV]++;
-        forbiddenTags[ID_CENTER]++;
-        forbiddenTags[ID_NOSCRIPT]++;
-        forbiddenTags[ID_NOFRAMES]++;
-        forbiddenTags[ID_BLOCKQUOTE]++;
-        forbiddenTags[ID_FORM]++;
-        forbiddenTags[ID_ISINDEX]++;
-        forbiddenTags[ID_HR]++;
-        forbiddenTags[ID_TABLE]++;
-        forbiddenTags[ID_FIELDSET]++;
-        forbiddenTags[ID_ADDRESS]++;
-        break;
     case ID_LABEL:
         forbiddenTags[ID_LABEL]++;
         break;
@@ -746,35 +715,6 @@
         //forbiddenTags[ID_SUP]--;
         forbiddenTags[ID_BASEFONT]--;
         break;
-    case ID_DIR:
-    case ID_MENU:
-        forbiddenTags[ID_P]--;
-        forbiddenTags[ID_H1]--;
-        forbiddenTags[ID_H2]--;
-        forbiddenTags[ID_H3]--;
-        forbiddenTags[ID_H4]--;
-        forbiddenTags[ID_H5]--;
-        forbiddenTags[ID_H6]--;
-        forbiddenTags[ID_UL]--;
-        forbiddenTags[ID_OL]--;
-        forbiddenTags[ID_DIR]--;
-        forbiddenTags[ID_MENU]--;
-        forbiddenTags[ID_PRE]--;
-        forbiddenTags[ID_PLAINTEXT]--;
-        forbiddenTags[ID_XMP]--;
-        forbiddenTags[ID_DL]--;
-        forbiddenTags[ID_DIV]--;
-        forbiddenTags[ID_CENTER]--;
-        forbiddenTags[ID_NOSCRIPT]--;
-        forbiddenTags[ID_NOFRAMES]--;
-        forbiddenTags[ID_BLOCKQUOTE]--;
-        forbiddenTags[ID_FORM]--;
-        forbiddenTags[ID_ISINDEX]--;
-        forbiddenTags[ID_HR]--;
-        forbiddenTags[ID_TABLE]--;
-        forbiddenTags[ID_FIELDSET]--;
-        forbiddenTags[ID_ADDRESS]--;
-        break;
     case ID_LABEL:
         forbiddenTags[ID_LABEL]--;
         break;
--- trunk/KDE/kdelibs/khtml/rendering/render_list.cpp #691135:691136
@@ -115,7 +115,8 @@
             break;
 
         if (currChild->style()->htmlHacks() && currChild->element() &&
-            (currChild->element()->id() == ID_UL || currChild->element()->id() == ID_OL))
+            (currChild->element()->id() == ID_UL || currChild->element()->id() == ID_OL ||
+             currChild->element()->id() == ID_DIR || currChild->element()->id() == ID_MENU))
             break;
 
         RenderObject* lineBox = getParentOfFirstLineBox(currChild, marker);
Comment 12 Maksim Orlovich 2008-01-08 18:21:26 UTC
*** Bug 155279 has been marked as a duplicate of this bug. ***