377677 - tree.xml cleanup

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Neil Deakin]

- some methods should be 'private'
- currentIndex fails if there is no view
- treecols shouldn't inherit from tree-base

Comment 1

[Neil Deakin]

Attachment: cleanup

Comment 2

[Neil Deakin]

Attachment: right patch

Comment 3

[Neil Deakin]

Attachment: now with testcases

Comment 4

[Neil Deakin]

Attachment: fix a bug with column widths in the tests

Comment 5

[Asaf Romano (gone)]

Comment on attachment 276793 [details] [diff] [review]
fix a bug with column widths in the tests

>+  // cursor navigation should not change the selection while editing
>+  synthesizeKeyExpectEvent("VK_DOWN", {}, tree, "!select", "key down with editing");
>+  is(tree.editingRow == 0 && tree.editingColumn == ecolumn && tree.currentIndex == ci,
>+                true, testid + "key down while editing");
>+  synthesizeKeyExpectEvent("VK_UP", {}, tree, "!select", "key up with editing");
>+  is(tree.editingRow == 0 && tree.editingColumn == ecolumn && tree.currentIndex == ci,
>+                true, testid + "key up while editing");
>+  synthesizeKeyExpectEvent("VK_PAGE_DOWN", {}, tree, "!select", "key page down with editing");
>+  is(tree.editingRow == 0 && tree.editingColumn == ecolumn && tree.currentIndex == ci,
>+                true, testid + "key page down while editing");
>+  synthesizeKeyExpectEvent("VK_PAGE_UP", {}, tree, "!select", "key page up with editing");
>+  is(tree.editingRow == 0 && tree.editingColumn == ecolumn && tree.currentIndex == ci,
>+                true, testid + "key page up while editing");
>+  synthesizeKeyExpectEvent("VK_HOME", {}, tree, "!select", "key home with editing");
>+  is(tree.editingRow == 0 && tree.editingColumn == ecolumn && tree.currentIndex == ci,
>+                true, testid + "key home while editing");
>+  synthesizeKeyExpectEvent("VK_END", {}, tree, "!select", "key end with editing");
>+  is(tree.editingRow == 0 && tree.editingColumn == ecolumn && tree.currentIndex == ci,
>+                true, testid + "key end while editing");
>+

could you put
>+  is(tree.editingRow == 0 && tree.editingColumn == ecolumn && tree.currentIndex == ci,
>+                true, testid + "key home while editing");

in a closure and save us some lines?

r=mano otherwise.

Comment 6

[Neil Deakin]

Attachment: address comment

Comment 7

[Neil Deakin]

Comment on attachment 276793 [details] [diff] [review]
fix a bug with column widths in the tests

The tree changes are very minor, so not really a high priority, but would like to get these tests in.

Comment 8

[Reed Loden [:reed]]

I backed this all completely out (though cvs was dumb and left the test files in the tree instead of removing them like it said it had scheduled them for...) to see if it is the cause of the Tp regression.

Comment 9

[Reed Loden [:reed]]

So, we had both a Tp and a Ts regression. Seems that the Tp regression happened far earlier than the Ts regression. The Ts regression turned out to be a tinderbox machine issue. So, sorry about backing you out. You should be fine for checking-in again once we have figured out the Tp regression (which we may have found now).

Comment 10

[Reed Loden [:reed]]

Enn, please reland this before Monday.

Comment 11

[Neil Deakin]

Checked this in again

Comment 12

[Tarmo Järvalt]

bug 401548 - regression caused by this one perhaps?