tree.xml cleanup

RESOLVED FIXED in mozilla1.9beta1

Status

()

Toolkit
XUL Widgets
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: Neil Deakin, Assigned: Neil Deakin)

Tracking

(Blocks: 1 bug)

unspecified
mozilla1.9beta1
x86
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

11 years ago
- some methods should be 'private'
- currentIndex fails if there is no view
- treecols shouldn't inherit from tree-base
(Assignee)

Updated

11 years ago
Blocks: 377253
(Assignee)

Comment 1

11 years ago
Created attachment 261870 [details] [diff] [review]
cleanup
(Assignee)

Comment 2

11 years ago
Created attachment 261871 [details] [diff] [review]
right patch
Attachment #261870 - Attachment is obsolete: true
(Assignee)

Updated

11 years ago
Target Milestone: --- → mozilla1.9beta2
(Assignee)

Comment 3

11 years ago
Created attachment 266213 [details] [diff] [review]
now with testcases
Attachment #261871 - Attachment is obsolete: true
(Assignee)

Comment 4

10 years ago
Created attachment 276793 [details] [diff] [review]
fix a bug with column widths in the tests
Attachment #266213 - Attachment is obsolete: true
Attachment #276793 - Flags: review?(mano)
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.
Attachment #276793 - Flags: review?(mano) → review+
(Assignee)

Comment 6

10 years ago
Created attachment 283052 [details] [diff] [review]
address comment
(Assignee)

Comment 7

10 years ago
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.
Attachment #276793 - Flags: approval1.9?

Updated

10 years ago
Attachment #276793 - Flags: approval1.9? → approval1.9+
(Assignee)

Updated

10 years ago
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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).
Enn, please reland this before Monday.
(Assignee)

Comment 11

10 years ago
Checked this in again
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED

Comment 12

10 years ago
bug 401548 - regression caused by this one perhaps?

Updated

10 years ago
Depends on: 401548
(Assignee)

Updated

9 years ago
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.