Bug 521542. Fix IsValidSibling to not lie when non-table-related siblings are involved. r=bernd
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 17 Nov 2009 17:50:04 -0500
changeset 34953 5a055cba72c19eaaaad2a732efe46d211a4e378d
parent 34952 9e3b7640f0b7577ec6ba5849d78af1dad33e8fd2
child 34954 4957ef02052301bc3857648902e04528fb62dd77
push idunknown
push userunknown
push dateunknown
reviewersbernd
bugs521542
milestone1.9.3a1pre
Bug 521542. Fix IsValidSibling to not lie when non-table-related siblings are involved. r=bernd
layout/base/nsCSSFrameConstructor.cpp
layout/reftests/bugs/521542-1-ref.xhtml
layout/reftests/bugs/521542-1.xhtml
layout/reftests/bugs/reftest.list
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -5889,29 +5889,44 @@ nsCSSFrameConstructor::IsValidSibling(ns
       const nsStyleDisplay* display = styleContext->GetStyleDisplay();
       aDisplay = display->mDisplay;
     }
     if (nsGkAtoms::menuFrame == parentType) {
       return
         (NS_STYLE_DISPLAY_POPUP == aDisplay) ==
         (NS_STYLE_DISPLAY_POPUP == siblingDisplay);
     }
-    switch (siblingDisplay) {
-    case NS_STYLE_DISPLAY_TABLE_COLUMN_GROUP:
-      return (NS_STYLE_DISPLAY_TABLE_COLUMN_GROUP == aDisplay);
-    case NS_STYLE_DISPLAY_TABLE_COLUMN:
-      return (NS_STYLE_DISPLAY_TABLE_COLUMN == aDisplay);
-    case NS_STYLE_DISPLAY_TABLE_CAPTION:
-      return (NS_STYLE_DISPLAY_TABLE_CAPTION == aDisplay);
-    default: // all of the row group types
-      return (NS_STYLE_DISPLAY_TABLE_HEADER_GROUP == aDisplay) ||
-             (NS_STYLE_DISPLAY_TABLE_ROW_GROUP    == aDisplay) ||
-             (NS_STYLE_DISPLAY_TABLE_FOOTER_GROUP == aDisplay) ||
-             (NS_STYLE_DISPLAY_TABLE_CAPTION      == aDisplay);
-    }
+    // To have decent performance we want to return false in cases in which
+    // reordering the two siblings has no effect on display.  To ensure
+    // correctness, we MUST return false in cases where the two siblings have
+    // the same desired parent type and live on different display lists.
+    // Specificaly, columns and column groups should only consider columns and
+    // column groups as valid siblings.  Captions should only consider other
+    // captions.  All other things should consider each other as valid
+    // siblings.  The restriction in the |if| above on siblingDisplay is ok,
+    // because for correctness the only part that really needs to happen is to
+    // not consider captions, column groups, and row/header/footer groups
+    // siblings of each other.  Treating a column or colgroup as a valid
+    // sibling of a non-table-related frame will just mean we end up reframing.
+    if ((siblingDisplay == NS_STYLE_DISPLAY_TABLE_CAPTION) !=
+        (aDisplay == NS_STYLE_DISPLAY_TABLE_CAPTION)) {
+      // One's a caption and the other is not.  Not valid siblings.
+      return PR_FALSE;
+    }
+
+    if ((siblingDisplay == NS_STYLE_DISPLAY_TABLE_COLUMN_GROUP ||
+         siblingDisplay == NS_STYLE_DISPLAY_TABLE_COLUMN) !=
+        (aDisplay == NS_STYLE_DISPLAY_TABLE_COLUMN_GROUP ||
+         aDisplay == NS_STYLE_DISPLAY_TABLE_COLUMN)) {
+      // One's a column or column group and the other is not.  Not valid
+      // siblings.
+      return PR_FALSE;
+    }
+
+    return PR_TRUE;
   }
   else if (nsGkAtoms::fieldSetFrame == parentType ||
            (nsGkAtoms::fieldSetFrame == grandparentType &&
             nsGkAtoms::blockFrame == parentType)) {
     // Legends can be sibling of legends but not of other content in the fieldset
     nsIAtom* sibType = aSibling->GetType();
     nsCOMPtr<nsIDOMHTMLLegendElement> legendContent(do_QueryInterface(aContent));
 
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/521542-1-ref.xhtml
@@ -0,0 +1,18 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+
+<head></head>
+
+<body>
+
+<table id="table">
+  BeginningOfTable
+  <tbody>
+    <tr>
+      <td>Cell</td>
+    </tr>
+  </tbody>
+  <span></span>
+</table>
+
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/521542-1.xhtml
@@ -0,0 +1,18 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+
+<head></head>
+
+<body onload="document.getElementById('table').firstChild.data = ' BeginningOfTable ';">
+
+<table id="table">
+
+  <tbody>
+    <tr>
+      <td>Cell</td>
+    </tr>
+  </tbody>
+  <span></span>
+</table>
+
+</body>
+</html>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1324,16 +1324,17 @@ fails-if(MOZ_WIDGET_TOOLKIT!="cocoa") ==
 == 512631-1.html 512631-1-ref.html
 == 513153-1a.html 513153-1-ref.html
 == 513153-1b.html 513153-1-ref.html
 == 513153-2a.html 513153-2-ref.html
 == 513153-2b.html 513153-2-ref.html
 == 513318-1.xul 513318-1-ref.xul
 != 513318-2.xul 513318-2-ref.xul
 != 513318-3.xul 513318-3-ref.xul
+== 521542-1.xhtml 521542-1-ref.xhtml
 == 520421-1.html 520421-1-ref.html
 == 520563-1.xhtml 520563-1-ref.xhtml
 == 521525-1.html 521525-1-ref.html
 == 521525-2.html 521525-2-ref.html
 == 521539-1.html 521539-1-ref.html
 == 521685-1.html 521685-1-ref.html
 == 523096-1.html 523096-1-ref.html
 == 523468-1.html 523468-1-ref.html