Bug 393970: grid columns don't line up if one <rows> block is scrollable, patch by Wladimir Palant <trev.moz@adblockplus.org>, r+sr=roc, a=beltzner
authorgavin@gavinsharp.com
Wed, 09 Apr 2008 15:15:59 -0700
changeset 14151 422b21561e1df882c77cca35281f3cf321964420
parent 14150 4aa2d168ffc43e6da4f5db85fe6437ff6ceb6364
child 14152 7f813317aed7a02fcef59916815a729c7474d9da
push idunknown
push userunknown
push dateunknown
reviewersbeltzner
bugs393970
milestone1.9pre
Bug 393970: grid columns don't line up if one <rows> block is scrollable, patch by Wladimir Palant <trev.moz@adblockplus.org>, r+sr=roc, a=beltzner
layout/xul/base/Makefile.in
layout/xul/base/src/grid/nsGridRowLeafLayout.cpp
layout/xul/base/test/Makefile.in
layout/xul/base/test/test_bug393970.xul
--- a/layout/xul/base/Makefile.in
+++ b/layout/xul/base/Makefile.in
@@ -39,10 +39,14 @@ DEPTH		= ../../..
 topsrcdir	= @top_srcdir@
 srcdir		= @srcdir@
 VPATH		= @srcdir@
 
 include $(DEPTH)/config/autoconf.mk
 
 DIRS		= public src
 
+ifdef MOZ_MOCHITEST
+DIRS		+= test
+endif
+
 include $(topsrcdir)/config/rules.mk
 
--- a/layout/xul/base/src/grid/nsGridRowLeafLayout.cpp
+++ b/layout/xul/base/src/grid/nsGridRowLeafLayout.cpp
@@ -139,56 +139,56 @@ nsGridRowLeafLayout::PopulateBoxSizes(ns
   PRInt32 index = 0;
   nsGrid* grid = GetGrid(aBox, &index);
   PRBool isHorizontal = IsHorizontal(aBox);
 
   // Our base class SprocketLayout is giving us a chance to change the box sizes before layout
   // If we are a row lets change the sizes to match our columns. If we are a column then do the opposite
   // and make them match or rows.
   if (grid) {
-   nsGridRow* column;
-   PRInt32 count = grid->GetColumnCount(isHorizontal); 
-   nsBoxSize* start = nsnull;
-   nsBoxSize* last = nsnull;
-   nsBoxSize* current = nsnull;
-   nsIBox* child = aBox->GetChildBox();
-   for (int i=0; i < count; i++)
-   {
-     column = grid->GetColumnAt(i,isHorizontal); 
+    nsGridRow* column;
+    PRInt32 count = grid->GetColumnCount(isHorizontal); 
+    nsBoxSize* start = nsnull;
+    nsBoxSize* last = nsnull;
+    nsBoxSize* current = nsnull;
+    nsIBox* child = aBox->GetChildBox();
+    for (int i=0; i < count; i++)
+    {
+      column = grid->GetColumnAt(i,isHorizontal); 
 
-     // make sure the value was computed before we use it.
-     // !isHorizontal is passed in to invert the behavior of these methods.
-     nscoord pref =
-       grid->GetPrefRowHeight(aState, i, !isHorizontal); // GetPrefColumnWidth
-     nscoord min = 
-       grid->GetMinRowHeight(aState, i, !isHorizontal);  // GetMinColumnWidth
-     nscoord max = 
-       grid->GetMaxRowHeight(aState, i, !isHorizontal);  // GetMaxColumnWidth
-     nscoord flex =
-       grid->GetRowFlex(aState, i, !isHorizontal);       // GetColumnFlex
-     nscoord left  = 0;
-     nscoord right  = 0;
-     grid->GetRowOffsets(aState, i, left, right, !isHorizontal); // GetColumnOffsets
-     nsIBox* box = column->GetBox();
-     PRBool collapsed = PR_FALSE;
-     nscoord topMargin = column->mTopMargin;
-     nscoord bottomMargin = column->mBottomMargin;
+      // make sure the value was computed before we use it.
+      // !isHorizontal is passed in to invert the behavior of these methods.
+      nscoord pref =
+        grid->GetPrefRowHeight(aState, i, !isHorizontal); // GetPrefColumnWidth
+      nscoord min = 
+        grid->GetMinRowHeight(aState, i, !isHorizontal);  // GetMinColumnWidth
+      nscoord max = 
+        grid->GetMaxRowHeight(aState, i, !isHorizontal);  // GetMaxColumnWidth
+      nscoord flex =
+        grid->GetRowFlex(aState, i, !isHorizontal);       // GetColumnFlex
+      nscoord left  = 0;
+      nscoord right  = 0;
+      grid->GetRowOffsets(aState, i, left, right, !isHorizontal); // GetColumnOffsets
+      nsIBox* box = column->GetBox();
+      PRBool collapsed = PR_FALSE;
+      nscoord topMargin = column->mTopMargin;
+      nscoord bottomMargin = column->mBottomMargin;
 
-     if (box) 
-       collapsed = box->IsCollapsed(aState);
+      if (box) 
+        collapsed = box->IsCollapsed(aState);
 
-     pref = pref - (left + right);
-     if (pref < 0)
-       pref = 0;
+      pref = pref - (left + right);
+      if (pref < 0)
+        pref = 0;
 
-     // if this is the first or last column. Take into account that
-     // our row could have a border that could affect our left or right
-     // padding from our columns. If the row has padding subtract it.
-     // would should always be able to garentee that our margin is smaller
-     // or equal to our left or right
+      // if this is the first or last column. Take into account that
+      // our row could have a border that could affect our left or right
+      // padding from our columns. If the row has padding subtract it.
+      // would should always be able to garentee that our margin is smaller
+      // or equal to our left or right
       PRInt32 firstIndex = 0;
       PRInt32 lastIndex = 0;
       nsGridRow* firstRow = nsnull;
       nsGridRow* lastRow = nsnull;
       grid->GetFirstAndLastRow(aState, firstIndex, lastIndex, firstRow, lastRow, !isHorizontal);
 
       if (i == firstIndex || i == lastIndex) {
         nsMargin offset = GetTotalMargin(aBox, isHorizontal);
@@ -213,99 +213,101 @@ nsGridRowLeafLayout::PopulateBoxSizes(ns
         {
           if (isHorizontal)
            right -= offset.right;
           else
            right -= offset.bottom;
         }
       }
     
-     // initialize the box size here 
-     max = PR_MAX(min, max);
-     pref = nsBox::BoundsCheck(min, pref, max);
+      // initialize the box size here 
+      max = PR_MAX(min, max);
+      pref = nsBox::BoundsCheck(min, pref, max);
    
-     current = new (aState) nsBoxSize();
-     current->pref = pref;
-     current->min = min;
-     current->max = max;
-     current->flex = flex;
-     current->bogus = column->mIsBogus;
-     current->left = left + topMargin;
-     current->right = right + bottomMargin;
-     current->collapsed = collapsed;
+      current = new (aState) nsBoxSize();
+      current->pref = pref;
+      current->min = min;
+      current->max = max;
+      current->flex = flex;
+      current->bogus = column->mIsBogus;
+      current->left = left + topMargin;
+      current->right = right + bottomMargin;
+      current->collapsed = collapsed;
 
-     if (!start) {
+      if (!start) {
         start = current;
         last = start;
-     } else {
+      } else {
         last->next = current;
         last = current;
-     }
+      }
 
-     if (child && !column->mIsBogus)
-       child = child->GetNextBox();
+      if (child && !column->mIsBogus)
+        child = child->GetNextBox();
 
-   }
-   aBoxSizes = start;
+    }
+    aBoxSizes = start;
   }
 
   nsSprocketLayout::PopulateBoxSizes(aBox, aState, aBoxSizes, aComputedBoxSizes, aMinSize, aMaxSize, aFlexes);
 }
 
 void
 nsGridRowLeafLayout::ComputeChildSizes(nsIBox* aBox,
                            nsBoxLayoutState& aState, 
                            nscoord& aGivenSize, 
                            nsBoxSize* aBoxSizes, 
                            nsComputedBoxSize*& aComputedBoxSizes)
 { 
   // see if we are in a scrollable frame. If we are then there could be scrollbars present
   // if so we need to subtract them out to make sure our columns line up.
   if (aBox) {
-     PRBool isHorizontal = aBox->IsHorizontal();
+    PRBool isHorizontal = aBox->IsHorizontal();
 
-     // go up the parent chain looking for scrollframes
-     aBox = aBox->GetParentBox();
-     nsIBox* scrollbox = nsGrid::GetScrollBox(aBox);
-       
-       nsCOMPtr<nsIScrollableFrame> scrollable = do_QueryInterface(scrollbox);
-       if (scrollable) {
-          nsMargin scrollbarSizes = scrollable->GetActualScrollbarSizes();
+    // go up the parent chain looking for scrollframes
+    nscoord diff = 0;
+    nsCOMPtr<nsIGridPart> parent;
+    nsIBox* parentBox;
+    GetParentGridPart(aBox, &parentBox, getter_AddRefs(parent));
+    while (parentBox) {
+      nsIBox* scrollbox = nsGrid::GetScrollBox(parentBox);
+      nsCOMPtr<nsIScrollableFrame> scrollable = do_QueryInterface(scrollbox);
+      if (scrollable) {
+        nsMargin scrollbarSizes = scrollable->GetActualScrollbarSizes();
 
-          nsRect ourRect(scrollbox->GetRect());
-          nsMargin padding(0,0,0,0);
-          scrollbox->GetBorderAndPadding(padding);
-          ourRect.Deflate(padding);
+        if (isHorizontal) {
+          diff += scrollbarSizes.left + scrollbarSizes.right;
+        } else {
+          diff += scrollbarSizes.top + scrollbarSizes.bottom;
+        }
+      }
 
-          nscoord diff;
-          if (isHorizontal) {
-            diff = scrollbarSizes.left + scrollbarSizes.right;
-          } else {
-            diff = scrollbarSizes.top + scrollbarSizes.bottom;
-          }
+      GetParentGridPart(parentBox, &parentBox, getter_AddRefs(parent));
+    }
 
-          if (diff > 0) {
-            aGivenSize += diff;
+    if (diff > 0) {
+      aGivenSize += diff;
 
-            nsSprocketLayout::ComputeChildSizes(aBox, aState, aGivenSize, aBoxSizes, aComputedBoxSizes);
+      nsSprocketLayout::ComputeChildSizes(aBox, aState, aGivenSize, aBoxSizes, aComputedBoxSizes);
+
+      aGivenSize -= diff;
 
-            aGivenSize -= diff;
+      nsComputedBoxSize* s    = aComputedBoxSizes;
+      nsComputedBoxSize* last = aComputedBoxSizes;
+      while(s)
+      {
+        last = s;
+        s = s->next;
+      }
+  
+      if (last) 
+        last->size -= diff;                         
 
-            nsComputedBoxSize* s    = aComputedBoxSizes;
-            nsComputedBoxSize* last = aComputedBoxSizes;
-            while(s)
-            {
-              last = s;
-              s = s->next;
-            }
-  
-            if (last) 
-                last->size -= diff;                         
-          }
-       }
+      return;
+    }
   }
       
   nsSprocketLayout::ComputeChildSizes(aBox, aState, aGivenSize, aBoxSizes, aComputedBoxSizes);
 
 }
 
 NS_IMETHODIMP
 nsGridRowLeafLayout::Layout(nsIBox* aBox, nsBoxLayoutState& aBoxLayoutState)
new file mode 100644
--- /dev/null
+++ b/layout/xul/base/test/Makefile.in
@@ -0,0 +1,51 @@
+#
+# ***** BEGIN LICENSE BLOCK *****
+# Version: MPL 1.1/GPL 2.0/LGPL 2.1
+#
+# The contents of this file are subject to the Mozilla Public License Version
+# 1.1 (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+# http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS IS" basis,
+# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+# for the specific language governing rights and limitations under the
+# License.
+#
+# The Original Code is mozilla.org code.
+#
+# The Initial Developer of the Original Code is
+# Wladimir Palant.
+# Portions created by the Initial Developer are Copyright (C) 2007
+# the Initial Developer. All Rights Reserved.
+#
+# Contributor(s):
+#
+# Alternatively, the contents of this file may be used under the terms of
+# either of the GNU General Public License Version 2 or later (the "GPL"),
+# or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+# in which case the provisions of the GPL or the LGPL are applicable instead
+# of those above. If you wish to allow use of your version of this file only
+# under the terms of either the GPL or the LGPL, and not to allow others to
+# use your version of this file under the terms of the MPL, indicate your
+# decision by deleting the provisions above and replace them with the notice
+# and other provisions required by the GPL or the LGPL. If you do not delete
+# the provisions above, a recipient may use your version of this file under
+# the terms of any one of the MPL, the GPL or the LGPL.
+#
+# ***** END LICENSE BLOCK *****
+
+DEPTH		= ../../../..
+topsrcdir	= @top_srcdir@
+srcdir		= @srcdir@
+VPATH		= @srcdir@
+relativesrcdir  = layout/xul/base/test
+
+include $(DEPTH)/config/autoconf.mk
+include $(topsrcdir)/config/rules.mk
+
+_TEST_FILES = 	test_bug393970.xul \
+		$(NULL)
+
+libs:: $(_TEST_FILES)
+	$(INSTALL) $^ $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/layout/xul/base/test/test_bug393970.xul
@@ -0,0 +1,88 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="/tests/SimpleTest/test.css" type="text/css"?>
+<?xml-stylesheet href="data:text/css,description {border: 1px solid black; padding: 2px;}" type="text/css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=393970
+-->
+<window title="Mozilla Bug 393970"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript" src="/MochiKit/packed.js" />
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"/>
+
+  <!-- test resuls are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=393970"
+     target="_blank">Mozilla Bug 393970</a>
+  </body>
+
+  <hbox flex="1" pack="start" style="visibility: hidden;">
+    <grid min-width="1000" width="1000">
+      <columns>
+        <column flex="1"/>
+        <column flex="2"/>
+        <column flex="3"/>
+      </columns>
+      <rows id="rows1">
+        <row>
+          <description id="cell11">test1</description>
+          <description id="cell12">test2</description>
+          <description id="cell13">test3</description>
+        </row>
+        <rows id="rows2" flex="1">
+          <row>
+            <description id="cell21">test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1</description>
+            <description id="cell22">test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2</description>
+            <description id="cell23">test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3</description>
+          </row>
+          <rows id="rows3">
+            <row>
+              <description id="cell31">test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1 test1</description>
+              <description id="cell32">test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2 test2</description>
+              <description id="cell33">test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3 test3</description>
+            </row>
+          </rows>
+        </rows>
+      </rows>
+    </grid>
+  </hbox>
+
+  <!-- test code goes here -->
+  <script type="application/javascript"><![CDATA[
+    /** Test for Bug 393970 **/
+
+    var tests = [
+      'overflow-x: hidden; overflow-y: hidden;',
+      'overflow-x: scroll; overflow-y: hidden;',
+      'overflow-x: hidden; overflow-y: scroll;',
+      'overflow-x: scroll; overflow-y: scroll;',
+    ];
+    var currentTest = -1;
+
+    function runNextTest() {
+      currentTest++;
+      if (currentTest >= tests.length) {
+        SimpleTest.finish();
+        return;
+      }
+
+      $("rows2").setAttribute("style", tests[currentTest]);
+      setTimeout(checkPositions, 0, tests[currentTest]);
+    }
+
+    function checkPositions(variant) {
+      for (var col = 1; col <= 3; col++) {
+        is($('cell1' + col).boxObject.x, $('cell2' + col).boxObject.x, "Cells (1," + col + ") and (2," + col + ") line up horizontally (with " + variant + ")");
+        is($('cell2' + col).boxObject.x, $('cell3' + col).boxObject.x, "Cells (2," + col + ") and (3," + col + ") line up horizontally (with " + variant + ")");
+      }
+      for (var row = 1; row <= 3; row++) {
+        is($('cell' + row + '1').boxObject.y, $('cell' + row + '2').boxObject.y, "Cells (" + row + ",1) and (" + row + ",2) line up vertically (with " + variant + ")");
+        is($('cell' + row + '2').boxObject.y, $('cell' + row + '3').boxObject.y, "Cells (" + row + ",2) and (" + row + ",3) line up vertically (with " + variant + ")");
+      }
+      runNextTest();
+    }
+ 
+    addLoadEvent(runNextTest);
+    SimpleTest.waitForExplicitFinish()
+   ]]></script>
+</window>