Last Comment Bug 604807 - Crash [@ nsHTMLSelectElement::GetOptionIndex] with QI to nsISelectElement
: Crash [@ nsHTMLSelectElement::GetOptionIndex] with QI to nsISelectElement
Status: RESOLVED FIXED
: crash, testcase
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla2.0b9
Assigned To: :Ms2ger (⌚ UTC+1/+2)
:
:
Mentors:
Depends on: 605271 619996
Blocks: 326633
  Show dependency treegraph
 
Reported: 2010-10-15 15:55 PDT by Jesse Ruderman
Modified: 2011-06-09 14:58 PDT (History)
8 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
betaN+
.13+
.13-fixed
.16+
.16-fixed


Attachments
testcase (crashes Firefox when loaded) (231 bytes, text/html)
2010-10-15 15:55 PDT, Jesse Ruderman
no flags Details
Patch v1 (2.31 KB, patch)
2010-11-05 02:08 PDT, :Ms2ger (⌚ UTC+1/+2)
bzbarsky: review+
Details | Diff | Splinter Review
Patch for checkin (3.32 KB, patch)
2010-11-05 14:57 PDT, :Ms2ger (⌚ UTC+1/+2)
dveditz: approval1.9.2.13+
dveditz: approval1.9.1.16+
Details | Diff | Splinter Review

Description Jesse Ruderman 2010-10-15 15:55:11 PDT
Created attachment 483659 [details]
testcase (crashes Firefox when loaded)
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2010-10-15 17:35:31 PDT
This is crap left over from XBL form controls.  We should make this interface noscript as a quick fix, imo.
Comment 2 :Ms2ger (⌚ UTC+1/+2) 2010-10-19 03:56:35 PDT
I have patches to remove nsISelectElement altogether, but I guess that won't fly for 2.0.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2010-10-19 08:31:43 PDT
Probably not.  Want to do a patch per comment 1?  ;)
Comment 4 Damon Sicore (:damons) 2010-10-19 12:00:04 PDT
We have a systematic problem we need to fix that will resolve this issue:  bug 605271.  It's a blocker, so minusing this one.
Comment 5 Daniel Veditz [:dveditz] 2010-10-20 10:14:48 PDT
It doesn't seem to crash the 1.9.2 branch, do we still want to make the interface [noscript] anyway just in case? Any others in the same family we should do the same to?
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2010-10-20 10:28:10 PDT
We should make this noscript on branch, yes.  imo.
Comment 7 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2010-10-24 06:56:49 PDT
This is fixed by Bug 605271.
Comment 8 Daniel Veditz [:dveditz] 2010-11-03 10:39:52 PDT
bug 605271 is a lot more than making the iface [noscript]. Is it necessary (and if so, is it a safe change for the branch?) or is there a quick IDL fix along the lines of comment 1?
Comment 9 :Ms2ger (⌚ UTC+1/+2) 2010-11-05 02:08:28 PDT
Created attachment 488440 [details] [diff] [review]
Patch v1

Stealing.
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2010-11-05 09:42:48 PDT
Comment on attachment 488440 [details] [diff] [review]
Patch v1

r=me
Comment 11 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2010-11-05 10:39:50 PDT
Do we want/need to take that on 2.0?
Comment 12 :Ms2ger (⌚ UTC+1/+2) 2010-11-05 14:57:39 PDT
Created attachment 488570 [details] [diff] [review]
Patch for checkin
Comment 13 Daniel Veditz [:dveditz] 2010-11-08 10:49:26 PST
Comment on attachment 488570 [details] [diff] [review]
Patch for checkin

Four addons tried to fool my search by doing this:
  const nsISelectElement = Components.interfaces.nsIDOMHTMLSelectElement;

(and then using nsISelectElement) but none use the real nsISelectElement so we look OK going this route.

Approved for 1.9.2.13 and 1.9.1.16, a=dveditz for release-drivers
Comment 15 Daniel Veditz [:dveditz] 2010-11-18 13:10:45 PST
Leaving open for trunk until bug 605271 has landed (or this patch is landed in the interim?). I believe checkin-needed referred only to the branches so I'm removing that keyword, but if we want this interim patch meanwhile it could be re-added.
Comment 16 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2010-12-14 16:50:35 PST
We should just land this patch on trunk.  Bug 605271 isn't going to make it.
Comment 17 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2010-12-16 04:37:27 PST
This should be blocking since 605271 isn't anymore.
Comment 18 Mounir Lamouri (:mounir) 2010-12-17 09:46:05 PST
Jonas, Johnny, this should block, isn't it?
Comment 19 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-12-17 10:24:16 PST
Yes
Comment 20 Mounir Lamouri (:mounir) 2010-12-17 19:02:29 PST
Pushed:
http://hg.mozilla.org/mozilla-central/rev/a01537c0bf19
Comment 21 christian 2011-01-04 15:42:21 PST
As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if  you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)
Comment 22 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-01-04 18:28:58 PST
Readding everything that got stripped ...

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