Crash [@ nsHTMLSelectElement::GetOptionIndex] with QI to nsISelectElement

RESOLVED FIXED in mozilla2.0b9

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: Ms2ger)

Tracking

(Blocks: 1 bug, {crash, testcase})

Trunk
mozilla2.0b9
crash, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 betaN+, blocking1.9.2 .13+, status1.9.2 .13-fixed, blocking1.9.1 .16+, status1.9.1 .16-fixed)

Details

(crash signature)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

7 years ago
Created attachment 483659 [details]
testcase (crashes Firefox when loaded)
This is crap left over from XBL form controls.  We should make this interface noscript as a quick fix, imo.
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
(Assignee)

Comment 2

7 years ago
I have patches to remove nsISelectElement altogether, but I guess that won't fly for 2.0.
Probably not.  Want to do a patch per comment 1?  ;)
We have a systematic problem we need to fix that will resolve this issue:  bug 605271.  It's a blocker, so minusing this one.
blocking2.0: ? → -
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?
We should make this noscript on branch, yes.  imo.
blocking1.9.1: ? → .15+
blocking1.9.2: ? → .12+
status1.9.1: --- → wanted
status1.9.2: --- → wanted
Depends on: 605271
This is fixed by Bug 605271.
Assignee: nobody → khuey
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?
Whiteboard: [fixed by 605271 on trunk]
(Assignee)

Comment 9

7 years ago
Created attachment 488440 [details] [diff] [review]
Patch v1

Stealing.
Assignee: khuey → Ms2ger
Status: NEW → ASSIGNED
Attachment #488440 - Flags: review?(bzbarsky)
Comment on attachment 488440 [details] [diff] [review]
Patch v1

r=me
Attachment #488440 - Flags: review?(bzbarsky) → review+
(Assignee)

Updated

7 years ago
Attachment #488440 - Flags: approval2.0?
Do we want/need to take that on 2.0?
(Assignee)

Comment 12

7 years ago
Created attachment 488570 [details] [diff] [review]
Patch for checkin
Attachment #488440 - Attachment is obsolete: true
Attachment #488440 - Flags: approval2.0?
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
Whiteboard: [fixed by 605271 on trunk] → [fixed by 605271 on trunk][c-n:1.9.1,1.9.2]
Attachment #488570 - Flags: approval1.9.2.13?
Attachment #488570 - Flags: approval1.9.1.16?
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
Attachment #488570 - Flags: approval1.9.2.13?
Attachment #488570 - Flags: approval1.9.2.13+
Attachment #488570 - Flags: approval1.9.1.16?
Attachment #488570 - Flags: approval1.9.1.16+
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f43d4cdbe257
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/88e47d219f9d
status1.9.1: wanted → .16-fixed
status1.9.2: wanted → .13-fixed
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.
Keywords: checkin-needed
Whiteboard: [fixed by 605271 on trunk][c-n:1.9.1,1.9.2] → [will be fixed by 605271 on trunk]
We should just land this patch on trunk.  Bug 605271 isn't going to make it.
Keywords: checkin-needed
Whiteboard: [will be fixed by 605271 on trunk]
(Assignee)

Updated

6 years ago
Attachment #488570 - Flags: approval2.0?
This should be blocking since 605271 isn't anymore.
blocking2.0: - → ?
Jonas, Johnny, this should block, isn't it?
Keywords: checkin-needed
Yes
blocking2.0: ? → beta9+
(Assignee)

Updated

6 years ago
Attachment #488570 - Flags: approval2.0?
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
(Assignee)

Updated

6 years ago
Depends on: 619996
Pushed:
http://hg.mozilla.org/mozilla-central/rev/a01537c0bf19
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Keywords: checkin-needed
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b9

Comment 21

6 years ago
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)
No longer blocks: 326633
blocking2.0: beta9+ → betaN+
No longer depends on: 605271, 619996
Keywords: crash, testcase
Keywords: crash, testcase
Readding everything that got stripped ...
Depends on: 605271, 619996

Updated

6 years ago
Blocks: 326633
Crash Signature: [@ nsHTMLSelectElement::GetOptionIndex]
You need to log in before you can comment on or make changes to this bug.