Closed Bug 479931 Opened 12 years ago Closed 11 years ago
Crash [@ ns
Tree Content View::Insert Row] with <xul:tree> and <option>
449 bytes, application/xhtml+xml
7.71 KB, text/plain
1.12 KB, patch
|Details | Diff | Splinter Review|
1.32 KB, patch
|Details | Diff | Splinter Review|
940 bytes, application/vnd.mozilla.xul+xml
Firefox crashes trying to do something with memory at 0x1c6ffffc.
Comment on attachment 364001 [details] [diff] [review] Like so? It's not the parent index that's wrong, it's the child index, and that's wrong because we're not allowing for the <div>. Mind you, I think the code for <optgroup> is wrong too, but that just fails "safe".
This is all just xbl form controls stuff. We should just remove it from the tree code.
Ah, there is already a function that takes into account content nodes that the tree ignores in order to determine proper indices, so use it here.
Assignee: nobody → tnikkel
Attachment #406901 - Flags: review?(neil)
Comment on attachment 406901 [details] [diff] [review] patch >- PRInt32 count = InsertRow(parentIndex, aIndexInContainer, aChild); >+ PRInt32 index = 0; >+ GetIndexInSubtree(aContainer, aChild, &index); >+ PRInt32 count = InsertRow(parentIndex, index, aChild); > if (mBoxObject) > mBoxObject->RowCountChanged(parentIndex + aIndexInContainer + 1, count); You meant to use index here too, right? r=me with that fixed.
Attachment #406901 - Flags: review?(neil) → review+
Whoops, thanks for catching that!
Fixed Neil's comment.
Even though 1.9.0 and 1.9.1 don't crash on the testcase, the same vulnerable code is on all branches, so I think we should take this on all branches.
blocking1.9.1: --- → ?
Is there any reason this hasn't landed on mozilla-central for testing yet?
Target Milestone: --- → mozilla1.9.3a1
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical?] → [sg:critical?][needs 1.9.2 approval or blocking]
Flags: blocking1.9.2? → blocking1.9.2+
Priority: -- → P1
Whiteboard: [sg:critical?][needs 1.9.2 approval or blocking] → [sg:critical?][needs 1.9.2 landing]
Comment on attachment 406942 [details] [diff] [review] patch v2 This is blocking 1.9.2 now, so it doesn't need approval to land there.
Is it possible to write a testcase that exercises the bug on the 1.9.1/1.9.0 branches? Otherwise QA won't be able to verify this.
I think the reason is doesn't crash on 1.9.0 and 1.9.1 is the switch from a nsVoidArray to a nsTArray. I haven't looked any more closely.
If you just need something that has observable differences before/after this patch on 1.9.1/1.9.0 this testcase will do. Clicking the button should insert an x between the a and b. Without the patch the x is inserted between the b and c.
Comment on attachment 406942 [details] [diff] [review] patch v2 Approved for 22.214.171.124 and 126.96.36.199, a=dveditz for release-drivers
Checking in nsTreeContentView.cpp; /cvsroot/mozilla/layout/xul/base/src/tree/src/nsTreeContentView.cpp,v <-- nsTreeContentView.cpp new revision: 1.71; previous revision: 1.70
(In reply to comment #17) > Created an attachment (id=410163) [details] > testcase with observable difference on 1.9.1/1.9.0 > > If you just need something that has observable differences before/after this > patch on 1.9.1/1.9.0 this testcase will do. > > Clicking the button should insert an x between the a and b. Without the patch > the x is inserted between the b and c. Verified using this testcase on 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:188.8.131.52) Gecko/20091102 Firefox/3.5.5. I see the difference in behavior between 184.108.40.206 and 220.127.116.11.
Verified using the same testcase on 1.9.0 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:18.104.22.168pre) Gecko/2009111921 GranParadiso/3.0.16pre (.NET CLR 3.5.30729) and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:22.214.171.124pre) Gecko/2009111916 GranParadiso/3.0.16pre.
Added crashtest to 1.9.1, 1.9.2, and mozilla-central. http://hg.mozilla.org/releases/mozilla-1.9.1/rev/976ec8c4d911 http://hg.mozilla.org/releases/mozilla-1.9.2/rev/914ae05fe0a2 http://hg.mozilla.org/mozilla-central/rev/260d768e55f4
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsTreeContentView::InsertRow]
Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336
Component: XP Toolkit/Widgets: XUL → XUL
You need to log in before you can comment on or make changes to this bug.