178604 - when I add new columns to a tree in foo.xul, they appear to the far left (instead of the far right) for existing profiles

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[(not reading, please use [e-mail] instead)]

when I add new columns to a tree in foo.xul, they appear to the far left 
(instead of the far right) for existing profiles

this blocks the spam bug.

fix in hand, r=varga

Comment 1

[(not reading, please use [e-mail] instead)]

Attachment: patch

Comment 2

[(not reading, please use [e-mail] instead)]

Attachment: patch with comment

Comment 3

[(not reading, please use [e-mail] instead)]

accepting

Comment 4

[Jan Varga [:janv]]

Comment on attachment 105301 [details] [diff] [review]
patch with comment

r=varga

Comment 5

[(not reading, please use [e-mail] instead)]

this has sr=bienvenu.

marking fixed.

thanks jan.

Comment 6

[(not reading, please use [e-mail] instead)]

adding neil, per http://bugzilla.mozilla.org/show_bug.cgi?id=169638#c86

Comment 7

[[e-mail]]

Comment on attachment 105301 [details] [diff] [review]
patch with comment

>+  // if there is no ordinal, come up with a big number, so that
>+  // the column shows up on the far right
>+  // see bug #178604
>   for (i = 0; i < cols.length; ++i)
>-    cols[i].setAttribute("ordinal", (i*2)+1);
>+    cols[i].setAttribute("ordinal", cols[i].getAttribute('ordinal') ? ((i*2)+1) : (((cols.length + i + 1) * 2) + 1));
This should be hasAttribute rather than getAttribute...

>     
>   // update the ordinal positions of splitters to even numbers, so that 
>   // they are in between columns
Ah, but will they be in between the far right columns? I think not.

Comment 8

[(not reading, please use [e-mail] instead)]

Attachment: supplimental patch

testing this patch now.

neil, can you review?

Comment 9

[(not reading, please use [e-mail] instead)]

that fix doesn't do quite what I want it to do.

my guess is after setting ordinals for the columns, we might have to assign 
ordinals to the splitters manually.

having some things have ordinal and others not, is problematic.

Comment 10

[(not reading, please use [e-mail] instead)]

Comment on attachment 105348 [details] [diff] [review]
supplimental patch

this is going to require some more thought.

Comment 11

[[e-mail]]

This broke splitters on trees that don't have a persisted ordinal.
E.g. if you have a new profile, open history and there are no splitters.

Comment 12

[[e-mail]]

Heh, and folderTree doesn't persist ordinals so it never works...

Comment 13

[[e-mail]]

Attachment: Proposed patch

Backs out the previous patch and uses a fix that works with splitters.

Comment 14

[(not reading, please use [e-mail] instead)]

> This broke splitters on trees that don't have a persisted ordinal.
> E.g. if you have a new profile, open history and there are no splitters.

yoinks!

re-assign to [e-mail], who has the right fix.

Comment 15

[(not reading, please use [e-mail] instead)]

Comment on attachment 105958 [details] [diff] [review]
Proposed patch

sr=sspitzer, thanks for fixing my regression.

Comment 16

[[e-mail]]

Attachment: Last patch still not quite right...

Apparently -moz-box-ordinal-group sorting isn't stable :-(
Column order in some trees (e.g. DOM inspector CSS rules) is wrong.

Comment 17

[[e-mail]]

Attachment: All headers should default to the same group...

By making all header elements (treecol, listheader, .tree-splitter) the same
ordinal group there's no problem with trees that don't set ordinals.

Comment 18

[(not reading, please use [e-mail] instead)]

Comment on attachment 106357 [details] [diff] [review]
All headers should default to the same group...

sr=sspitzer

assuming that large int is OK on all platforms.

thinking about loud,

that's 2^31 - 1 (or 2?)

so it should be safe.

Comment 19

[sairuh (rarely reading bugmail)]

i noticed how this affects order of columns in xul file picker (bug 180023).

Comment 20

[[e-mail]]

Patch was checked in (a fortnight ago!) by timeless.