Last Comment Bug 344085 - still content can implement tree views
: still content can implement tree views
Status: RESOLVED FIXED
[sg:critical?]
: verified1.8.0.7, verified1.8.1
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: All All
: -- critical (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-07-10 01:15 PDT by shutdown
Modified: 2008-07-31 03:21 PDT (History)
5 users (show)
darin.moz: blocking1.8.1+
dveditz: blocking1.8.0.7+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (1.22 KB, application/vnd.mozilla.xul+xml)
2006-07-10 01:29 PDT, shutdown
no flags Details
fix (3.02 KB, patch)
2006-07-13 21:48 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
bzbarsky: review+
bzbarsky: superreview+
dveditz: approval1.8.0.7+
dbaron: approval1.8.1+
Details | Diff | Splinter Review
Fix as per comment #3 (5.34 KB, patch)
2006-07-22 17:29 PDT, neil@parkwaycc.co.uk
no flags Details | Diff | Splinter Review
Real fix as per comment #3 (8.87 KB, patch)
2006-07-25 15:23 PDT, neil@parkwaycc.co.uk
roc: review+
roc: superreview+
Details | Diff | Splinter Review

Description shutdown 2006-07-10 01:15:58 PDT
See bug 326501. nsINativeTreeView check, introduced by said bug, can be circumvented by using treeElement.boxObject.setPropertyAsSupports().
Comment 1 shutdown 2006-07-10 01:29:28 PDT
Created attachment 228655 [details]
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) (Exited; email my personal email if necessary) 2006-07-13 21:48:32 PDT
Created attachment 229212 [details] [diff] [review]
fix

Fairly simple.
Comment 3 Boris Zbarsky [:bz] 2006-07-16 09:34:36 PDT
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) (Exited; email my personal email if necessary) 2006-07-19 23:09:50 PDT
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) (Exited; email my personal email if necessary) 2006-07-20 11:02:48 PDT
checked in on trunk
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-20 11:03:34 PDT
Comment on attachment 229212 [details] [diff] [review]
fix

need this on branches
Comment 7 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-20 11:11:09 PDT
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 neil@parkwaycc.co.uk 2006-07-22 17:29:51 PDT
Created attachment 230292 [details] [diff] [review]
Fix as per comment #3

Untested as yet, though; might leak or worse...
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-23 00:09:07 PDT
So when nsTreeBodyFrame creates the view in EnsureView(), how does nsTreeBoxObject find out about it so nsTreeBoxObject::GetView() works?
Comment 10 neil@parkwaycc.co.uk 2006-07-23 17:25:45 PDT
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 neil@parkwaycc.co.uk 2006-07-25 15:23:55 PDT
Created attachment 230643 [details] [diff] [review]
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 [:bz] 2006-07-27 19:29:59 PDT
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) (Exited; email my personal email if necessary) 2006-07-28 08:43:21 PDT
Comment on attachment 229212 [details] [diff] [review]
fix

branch security fixes
Comment 14 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-28 08:46:18 PDT
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-7 (review requests must explain patch) 2006-07-28 10:55:44 PDT
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) (Exited; email my personal email if necessary) 2006-07-28 11:28:45 PDT
Checked into 1.8.1 branch.
Comment 17 Daniel Veditz [:dveditz] 2006-08-09 14:35:52 PDT
Comment on attachment 229212 [details] [diff] [review]
fix

approved for 1.8.0 branch, a=dveditz for drivers
Comment 18 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-08-09 22:27:44 PDT
Fixed on 1.8.0 branch
Comment 19 Jay Patel [:jay] 2006-08-24 16:15:11 PDT
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

Note You need to log in before you can comment on or make changes to this bug.