192175 - in trees, in a treecol with persist="sortActive" doesn't work in appropiate manner

Status: RESOLVED FIXED

(Core :: XUL, defect, P3)

Description

[Ariel Fatecha]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2.1) Gecko/20021130
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2.1) Gecko/20021130

  persist="sortActive" would keep the value of an attribute sortActive in the
treecol, and that does it. The problem is just 1 (one) column can has the
attribute sortActive="true" in the same time (that has sense), and tag's parent
would see and change it (but doesn't occur).
  in fact, sortActive need the same treatment as sortDirection attribute.
  now, if a tree has 1 treecols and 2 treecol inside the last, I couldn't have a
persist="sortActive" in both treecol and no sortActive attribute, because
persist nothing.
  I couldn't have persist="sortActive" in both and a sortActive="true" inside
one treecol (suppose the first column), because when my last click was in the
second persist nothing (reasonable).
  I couldn't have persist="sortActive" and sortActive="true" in both, because
don't work, reasonable: just one treecol can take the sortActive attribute.
  In the browser, people write code which takes around the problem.("circunda el
problema": if you know spanish).
  If one person want to have always the same column with sortActive="true", just
has write it in one treecol (in xul) and don't persist="sortActive".
  the file "nsXULTreeBuilder.cpp" does the work, in ::Sort. 
  So I have a proposed patch, and a testcase.
  

Reproducible: Always

Steps to Reproduce:
1.open the testcase
2.try previous lines I wrote


Actual Results:  
don't good persist and sort.

Expected Results:  
persist the last column clicked like default for sort.

  the patch consist in add, in Sort's method (inside
content/xul/templates/src/nsXULTreeBuilder.cpp), treatment for sortActive
attribute in the same manner it's doing with sortDirection attribute.
  in a second, I will post my attachments. 
  .dif it's the proposed patch.
  [english lenguage is hard to me, sorry]

Comment 1

[Ariel Fatecha]

Attachment: it's a .rdf file to use with the treeTestCase.xul

it's for treeTestCase.xul

Comment 2

[Ariel Fatecha]

Attachment: it's for make test about this bug

it's for make easy testing

Comment 3

[Ariel Fatecha]

Attachment: proposed patch in diff

proposed path in diff.
over:content/xul/templates/src/nsXULTreeBuilder.cpp

Comment 4

[Boris Zbarsky [:bzbarsky]]

So... why did you cc me?  I don't do XUL and I particularly don't do trees....

Comment 5

[Ariel Fatecha]

  I did know that XUL isn't your field, but in the two previous bugs that I
reported you seemed to be one of the few who actually read them and did
something about them.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 113744 [details] [diff] [review]
proposed patch in diff

Yeah.... that just means I actually read my bugmail.

If you could attach this as a diff -u (much more readable) that would be good. 
;)

Comment 7

[Boris Zbarsky [:bzbarsky]]

Make that -u10, actually.  Patches with no context are very hard to review...

Comment 8

[Ariel Fatecha]

Attachment: with diff -u10

Comment 9

[Ariel Fatecha]

looks better, you 're right, thanks

Comment 10

[Jan Varga [:janv]]

Comment on attachment 113801 [details] [diff] [review]
with diff -u10

looks good
r=varga

Comment 11

[jag (Peter Annema)]

Comment on attachment 113801 [details] [diff] [review]
with diff -u10

sr=jag

Comment 12

[Asa Dotzler [:asa]]

Does this impact the Mozilla suite? If not, then let's hold this until we open
for 1.4alpha (real soon now). 

Comment 13

[Jan Varga [:janv]]

It's not so important.
Moving to correct milestone.

Comment 14

[Jan Varga [:janv]]

checked in
thanks for this patch