344085 - still content can implement tree views

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[shutdown]

See bug 326501. nsINativeTreeView check, introduced by said bug, can be circumvented by using treeElement.boxObject.setPropertyAsSupports().

Comment 1

[shutdown]

Attachment: testcase

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1b1) Gecko/20060709 BonEcho/2.0b1
TB20774884Z

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Fairly simple.

Comment 3

[Boris Zbarsky [:bzbarsky]]

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

This works, but really we should just be storing this as a member, not a property, imo...  The property mechanism is there to allow storage of random things, so taking up parts of the namespace is not so cool.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah. Unfortunately fixing the usage of the "view" property requires either adding a new private interface to nsTreeBoxObject or doing deeper changes than would be good for branch. So I'll go with this patch for trunk and branch, for now.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in on trunk

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

need this on branches

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

guess I'll wait for approval requests until this has been on trunk for a couple of days

Comment 8

[[e-mail]]

Attachment: Fix as per comment #3

Untested as yet, though; might leak or worse...

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

So when nsTreeBodyFrame creates the view in EnsureView(), how does nsTreeBoxObject find out about it so nsTreeBoxObject::GetView() works?

Comment 10

[[e-mail]]

Comment on attachment 230292 [details] [diff] [review]
Fix as per comment #3

>         // Hook up the view.
>         if (view)
>-          SetView(view);
>+          mTreeBoxObject->SetView(view);
The solution eluded me for a while, until I came up with this flash of inspiration; assuming that you don't have malformed xul, when you don't actually have a box object, so I'll need to null-check.

Comment 11

[[e-mail]]

Attachment: Real fix as per comment #3

I had to move some of the view creation logic into the box object because people expect to be able to get a view from an invisible (collapsed) tree.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Neil, can you just get roc to r+sr this?  I'm not going to be able to get to it in the near future (several weeks).

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

branch security fixes

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 230643 [details] [diff] [review]
Real fix as per comment #3

Looks good, thanks. I think we should take this on trunk only, though, since it's somewhat invasive.

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

a=dbaron on behalf of drivers.  Please check in to MOZILLA_1_8_BRANCH and mark fixed1.8.1 once you have done so.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked into 1.8.1 branch.

Comment 17

[Daniel Veditz [:dveditz]]

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

approved for 1.8.0 branch, a=dveditz for drivers

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Fixed on 1.8.0 branch

Comment 19

[Jay Patel [:jay]]

v.fixed on both branches with 8/24 nightly builds, no crash with testcase:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.7pre) Gecko/20060824 Firefox/1.5.0.7pre
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1b2) Gecko/20060824 BonEcho/2.0b2