From e2a623f614514a8b3c2d872275943d0c5662c920 Mon Sep 17 00:00:00 2001 From: "reed@reedloden.com" Date: Fri, 4 Jan 2008 22:47:42 -0800 Subject: [PATCH] Bug 407453 - "Column reordering broken when ordinal > 9 (comparing numbers as strings)" [p=skrulx@gmail.com (Steve Krulewitz) r=Enn a1.9=schrep] --- toolkit/content/tests/widgets/Makefile.in | 1 + .../widgets/test_tree_column_reorder.xul | 77 ++++++++++ toolkit/content/tests/widgets/tree_shared.js | 142 ++++++++++++++++++ toolkit/content/widgets/tree.xml | 37 +++-- 4 files changed, 246 insertions(+), 11 deletions(-) create mode 100644 toolkit/content/tests/widgets/test_tree_column_reorder.xul diff --git a/toolkit/content/tests/widgets/Makefile.in b/toolkit/content/tests/widgets/Makefile.in index f4e8120bc22..fa654f0a2ab 100644 --- a/toolkit/content/tests/widgets/Makefile.in +++ b/toolkit/content/tests/widgets/Makefile.in @@ -79,6 +79,7 @@ _TEST_FILES = test_bug360220.xul \ test_tree_single.xul \ test_tree_hier.xul \ test_tree_hier_cell.xul \ + test_tree_column_reorder.xul \ tree_shared.js \ test_textbox_number.xul \ xul_selectcontrol.js \ diff --git a/toolkit/content/tests/widgets/test_tree_column_reorder.xul b/toolkit/content/tests/widgets/test_tree_column_reorder.xul new file mode 100644 index 00000000000..7e85070d1a9 --- /dev/null +++ b/toolkit/content/tests/widgets/test_tree_column_reorder.xul @@ -0,0 +1,77 @@ + + + + + + + + + + + + + diff --git a/toolkit/content/tests/widgets/tree_shared.js b/toolkit/content/tests/widgets/tree_shared.js index f1cda68bd33..5ede676e1b9 100644 --- a/toolkit/content/tests/widgets/tree_shared.js +++ b/toolkit/content/tests/widgets/tree_shared.js @@ -986,6 +986,148 @@ function testtag_tree_TreeSelection_State(tree, testid, current, selected, viewi is(compareArrays(selected, actualSelected), true, testid + " range selection [" + selected + "]"); } +function testtag_tree_column_reorder() +{ + // Make sure the tree is scrolled into the view, otherwise the test will + // fail + var testframe = window.parent.document.getElementById("testframe"); + if (testframe) { + testframe.scrollIntoView(); + } + + var tree = document.getElementById("tree-column-reorder"); + var numColumns = tree.columns.count; + + var reference = []; + for (var i = 0; i < numColumns; i++) { + reference.push("col_" + i); + } + + // Drag the first column to each position + for (var i = 0; i < numColumns - 1; i++) { + synthesizeColumnDrag(tree, i, i + 1, true); + arrayMove(reference, i, i + 1, true); + checkColumns(tree, reference, "drag first column right"); + } + + // And back + for (var i = numColumns - 1; i >= 1; i--) { + synthesizeColumnDrag(tree, i, i - 1, false); + arrayMove(reference, i, i - 1, false); + checkColumns(tree, reference, "drag last column left"); + } + + // Drag each column one column left + for (var i = 1; i < numColumns; i++) { + synthesizeColumnDrag(tree, i, i - 1, false); + arrayMove(reference, i, i - 1, false); + checkColumns(tree, reference, "drag each column left"); + } + + // And back + for (var i = numColumns - 2; i >= 0; i--) { + synthesizeColumnDrag(tree, i, i + 1, true); + arrayMove(reference, i, i + 1, true); + checkColumns(tree, reference, "drag each column right"); + } + + // Drag each column 5 to the right + for (var i = 0; i < numColumns - 5; i++) { + synthesizeColumnDrag(tree, i, i + 5, true); + arrayMove(reference, i, i + 5, true); + checkColumns(tree, reference, "drag each column 5 to the right"); + } + + // And to the left + for (var i = numColumns - 6; i >= 5; i--) { + synthesizeColumnDrag(tree, i, i - 5, false); + arrayMove(reference, i, i - 5, false); + checkColumns(tree, reference, "drag each column 5 to the left"); + } + + // Test that moving a column after itself does not move anything + synthesizeColumnDrag(tree, 0, 0, true); + checkColumns(tree, reference, "drag to itself"); + is(document.treecolDragging, null, "drag to itself completed"); + + SimpleTest.finish(); + +} + +function synthesizeColumnDrag(aTree, aMouseDownColumnNumber, aMouseUpColumnNumber, aAfter) +{ + var columns = getSortedColumnArray(aTree); + + var down = columns[aMouseDownColumnNumber].element; + var up = columns[aMouseUpColumnNumber].element; + + // Target the initial mousedown in the middle of the column header so we + // avoid the extra hit test space given to the splitter + var columnWidth = down.boxObject.width; + var splitterHitWidth = columnWidth / 2; + synthesizeMouse(down, splitterHitWidth, 0, { type: "mousedown"}); + + var offsetX = 0; + if (aAfter) { + offsetX = columnWidth; + } + + if (aMouseUpColumnNumber > aMouseDownColumnNumber) { + for (var i = aMouseDownColumnNumber; i <= aMouseUpColumnNumber; i++) { + var move = columns[i].element; + synthesizeMouse(move, offsetX, 0, { type: "mousemove"}); + } + } + else { + for (var i = aMouseDownColumnNumber; i >= aMouseUpColumnNumber; i--) { + var move = columns[i].element; + synthesizeMouse(move, offsetX, 0, { type: "mousemove"}); + } + } + + synthesizeMouse(up, offsetX, 0, { type: "mouseup"}); +} + +function arrayMove(aArray, aFrom, aTo, aAfter) +{ + var o = aArray.splice(aFrom, 1)[0]; + if (aTo > aFrom) { + aTo--; + } + + if (aAfter) { + aTo++; + } + + aArray.splice(aTo, 0, o); +} + +function getSortedColumnArray(aTree) +{ + var columns = aTree.columns; + var a = []; + for (var i = 0; i < columns.length; i++) { + a.push(columns.getColumnAt(i)); + } + + a.sort(function(a, b) { + var o1 = parseInt(a.element.getAttribute("ordinal")); + var o2 = parseInt(b.element.getAttribute("ordinal")); + return o1 - o2; + }); + return a; +} + +function checkColumns(aTree, aReference, aMessage) +{ + var columns = getSortedColumnArray(aTree); + var ids = []; + columns.forEach(function(e) { + ids.push(e.element.id); + }); + is(compareArrays(ids, aReference), true, aMessage); +} + function mouseOnCell(tree, row, column, testname) { var x = {}, y = {}, width = {}, height = {}; diff --git a/toolkit/content/widgets/tree.xml b/toolkit/content/widgets/tree.xml index 97a5e1a0456..3e6c497442c 100644 --- a/toolkit/content/widgets/tree.xml +++ b/toolkit/content/widgets/tree.xml @@ -170,7 +170,7 @@ var i; var cols = []; var col = this.columns.getColumnFor(aColBefore); - if (aColBefore.ordinal < aColMove.ordinal) { + if (parseInt(aColBefore.ordinal) < parseInt(aColMove.ordinal)) { if (aBefore) cols.push(aColBefore); for (col = col.getNext(); col.element != aColMove; @@ -179,7 +179,7 @@ aColMove.ordinal = cols[0].ordinal; for (i = 0; i < cols.length; ++i) - cols[i].ordinal += 2; + cols[i].ordinal = parseInt(cols[i].ordinal) + 2; } else if (aColBefore.ordinal != aColMove.ordinal) { if (!aBefore) cols.push(aColBefore); @@ -189,7 +189,7 @@ aColMove.ordinal = cols[0].ordinal; for (i = 0; i < cols.length; ++i) - cols[i].ordinal -= 2; + cols[i].ordinal = parseInt(cols[i].ordinal) - 2; } ]]> @@ -1194,16 +1194,31 @@ // remove insertbefore/after attributes var before = col.mTargetCol.hasAttribute("insertbefore"); col.mTargetCol.removeAttribute(before ? "insertbefore" : "insertafter"); - if (before) { - var sib = col.mTargetCol._previousVisibleColumn; - if (sib) - sib.removeAttribute("insertafter"); + + var sib = col.mTargetCol._previousVisibleColumn; + if (before && sib) { + sib.removeAttribute("insertafter"); } - - // move the column - if (col != col.mTargetCol) + + // Move the column only if it will result in a different column + // ordering + var move = true; + + // If this is a before move and the previous visible column is + // the same as the column we're moving, don't move + if (before && col == sib) { + move = false; + } + else if (!before && col == col.mTargetCol) { + // If this is an after move and the column we're moving is + // the same as the target column, don't move. + move = false; + } + + if (move) { col.parentNode.parentNode._reorderColumn(col, col.mTargetCol, before); - + } + // repaint to remove lines col.parentNode.parentNode.treeBoxObject.invalidate();