415257 - Editable trees: input sized wrongly and rows jumping with flexible and non-flexible treecols

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Malte Kraus]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008020104 Minefield/3.0b3pre
Build Identifier: 

With an editable trees consisting of a treecol with flex="1" and one with flex="0", the edit field is sized wrongly and the flexible column jumps around when the inflexible column is edited.

I'll attach a testcase and a patch immediately.

Reproducible: Always

Comment 1

[Malte Kraus]

Attachment: testcase

Comment 2

[Malte Kraus]

Attachment: trivial fix

Comment 3

[Neil Deakin]

Did you want someone to review this patch?

Comment 4

[Malte Kraus]

Well, I have no idea what to put into all these fields for the patch to be reviewed. But since I had already found the problem when trying to find an error in my code, I figured I could also attach the patch.

Comment 5

[Paul Rouget [:paul]]

Attachment: patch

tree.xml:
The input field must be displayed *after* resizing.

nsTreeBodyFrame.cpp:
GetCoordsForCellItem must consider scrolled position.
EnsureCellIsVisible must not remove the column picker width (since cells of the last column use its space).

Comment 6

[Neil Deakin]

Comment on attachment 307239 [details] [diff] [review]
patch

Not sure what the change in  EnsureCellIsVisible is for. Maybe another Neil should review this.

Comment 7

[Paul Rouget [:paul]]

In the specific case of the last column, the real width of the cell includes the columnPicker width. Currently, if you use EnsureCellIsVisible with such a cell, the page is not enough scrolled (missing the columnPicker size), so the input field overlap the scrollbar (because the input field use the good with of the cell, with the columnPicker width).

Comment 8

[[e-mail]]

Comment on attachment 307239 [details] [diff] [review]
patch

>   nscoord currX = mInnerBox.x;
I think you should subtract the horizontal position here, rather than alter.

>-  if (col->IsLastVisible(this))
>-    columnWidth -= mAdjustWidth; // this is one case we don't want to adjust
I can't remember why I wrote this, but I agree it's wrong - I created a cell-based selection and found that this fixed selecting the rightmost cell.

>+            input.hidden = false;
I didn't test this but it looks right.

Comment 9

[Paul Rouget [:paul]]

(In reply to comment #8)
> (From update of attachment 307239 [details] [diff] [review])
> >   nscoord currX = mInnerBox.x;
> I think you should subtract the horizontal position here, rather than alter.
Yes, you're right.

> >-  if (col->IsLastVisible(this))
> >-    columnWidth -= mAdjustWidth; // this is one case we don't want to adjust
> I can't remember why I wrote this, but I agree it's wrong - I created a
> cell-based selection and found that this fixed selecting the rightmost cell.
> 
> >+            input.hidden = false;
> I didn't test this but it looks right.
If it's done before being resized, the old size could cause some weird scroll behavior.

Comment 10

[Paul Rouget [:paul]]

Attachment: patch v1

Comment 11

[[e-mail]]

Comment on attachment 307905 [details] [diff] [review]
patch v1

I tried this with a columnpicker and no scrollbar, and with a scrollbar and no columnpicker, and with neither.

Comment 12

[Paul Rouget [:paul]]

Who should I ask super review to ?

Comment 13

[Neil Deakin]

Comment on attachment 307905 [details] [diff] [review]
patch v1

I'm not a superreviewer but we can ask the other Neil

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 307905 [details] [diff] [review]
patch v1

a1.9=beltzner

Comment 15

[Reed Loden [:reed]]

Checking in layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp,v  <--  nsTreeBodyFrame.cpp
new revision: 1.350; previous revision: 1.349
done
Checking in toolkit/content/widgets/tree.xml;
/cvsroot/mozilla/toolkit/content/widgets/tree.xml,v  <--  tree.xml
new revision: 1.55; previous revision: 1.54
done