122794 - Outliner columns ordered incorrectly

Status: RESOLVED FIXED

(Core :: XUL, defect, P3)

Description

[Patty Gilbert]

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; Q312461)
BuildID:    0.9.7 Release

Starting with 0.9.7 release, some of my outliners get their columns swapped.  
In the affected outliners, the first and fourth columns get swapped.  The 
attachment shows an example of this.  When I load this XUL file, the columns 
show as "ColumnFour", "ColumnTwo", "ColumnThree", "ColumnOne", etc.

Reproducible: Always
Steps to Reproduce:
1. Bring up the attached XUL file in Mozilla 0.9.7.
2. Compare the order of the columns on screen to their order in the XUL file.


Actual Results:  Columns 1 and 4 are swapped.

Expected Results:  Column order should follow the order in the XUL file.

Comment 1

[Patty Gilbert]

Attachment: Demonstrates column reordering problem.

Comment 2

[Marc Boullet]

WFM 2002012903 NT4

Comment 3

[Alfonso Martinez]

WFM 2002-01-30-03  Win98

Comment 4

[Jan Varga [:janv]]

I don't have 0.9.7 by hand, but WFM with tip build.

Comment 5

[John Morrison]

There is _something_ odd occurring here, along the lines of what Patty notes.
At first, the first attached example worked for me with a tip build. But I 
updated to the current syntax (this is correct, right?), and when there are 
six (and only when there are six) columns, the first and fourth columns are
swapped (at least the labels are swapped).

<?xml version="1.0"?>
<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
<window  id="test_window5"
    xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
    xmlns:html="http://www.w3.org/1999/xhtml"
>
  <outliner flex="1">
    <outlinercols>
      <outlinercol id="c1" label="One"   flex="1"/>
      <outlinercol id="c2" label="Two"   flex="1"/>
      <outlinercol id="c3" label="Three" flex="1"/>
      <outlinercol id="c4" label="Four"  flex="1"/>
      <outlinercol id="c5" label="Five"  flex="1"/>
      <outlinercol id="c6" label="Six"   flex="1"/>
<!--
      <outlinercol id="c7" label="Seven" flex="1"/>
      <outlinercol id="c8" label="Eight" flex="1"/>
      <outlinercol id="c9" label="Nine" flex="1"/>
      <outlinercol id="c10" label="Ten" flex="1"/>
      <outlinercol id="c11" label="Eleven" flex="1"/>
-->
    </outlinercols>
    <outlinerchildren/>
  </outliner>
</window>

Comment 6

[John Morrison]

Attachment: updated example

I should also note that I was testing this file against a fresh default profile
(i.e., no previously persisted information in the localstore)

Comment 7

[:deanis74]

Weird.  They appear in the proper order in the column picker.  The columns are
switched, not just the labels.  Selecting One from the column picker affects the
"One" column.

Comment 8

[jag (Peter Annema)]

Jan, should you own this bug?

Comment 9

[Jan Varga [:janv]]

Dean's comment indicates a problem with ordinal box layout I think
Maybe hewitt should look.
Taking for now.

Comment 10

[[e-mail]]

DOM Inspector gives the ordinal attributes as follows:
7 One
3 Two
5 Three
1 Four
9 Five
11 Siz

Comment 11

[Jan Varga [:janv]]

good catch neil, thanks!

Comment 12

[Jan Varga [:janv]]

I blame quick sort, since it is an unstable algorithm.
So when columns didn't get their ordinal attribute yet it might reshuffle
their order.

Comment 13

[Jan Varga [:janv]]

Yeah, we need some stable algorithm.

Comment 14

[:deanis74]

Which algorithm, exactly?

Comment 15

[Jan Varga [:janv]]

I guess, the best of all stable ones :)
I tried bubble sort and it fixed this bug.
Now I'm trying heap sort.

Comment 16

[:deanis74]

Sorry, what I meant was were's the code that uses the quick sort?  (I don't know
how I expected you to know what I meant!)

Comment 17

[Jan Varga [:janv]]

nsContainerBox.cpp

Comment 18

[Jan Varga [:janv]]

Attachment: fix

use selection sort instead of quick sort, sigh

Comment 19

[Jan Varga [:janv]]

hewitt, could you r/sr= ?

Comment 20

[Joe Hewitt (gone)]

Comment on attachment 67577 [details] [diff] [review]
fix

You need to remove the declaration of BoxOrderSortComparison as well.

Comment 21

[Jan Varga [:janv]]

Attachment: revised patch

Comment 22

[Joe Hewitt (gone)]

Comment on attachment 67640 [details] [diff] [review]
revised patch

sr=hewitt

Comment 23

[Brian Ryner (not reading)]

For our current uses of box ordinals, speed isn't going to be a problem here
because we are dealing with small numbers of elements.  Having the sort be
stable is more important.

r=bryner.

Comment 24

[Jan Varga [:janv]]

fixed