Closed Bug 1228670 Opened 4 years ago Closed 4 years ago

Crash [@ GetMaxOptionBSize] — <optgroup style="display: list-item; list-style: inside;"></optgroup>

Categories

(Core :: Layout: Form Controls, defect, critical)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla45
Tracking Status
firefox43 --- unaffected
firefox44 + fixed
firefox45 + fixed
b2g-v2.5 --- fixed

People

(Reporter: jruderman, Assigned: mats)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase)

Attachments

(5 files)

Attached file testcase
Debug:
  [@ GetMaxOptionBSize]
  Stack attached

Nightly:
  [@ GetMaxOptionBSize]
  bp-e49b0664-a7f5-4651-9c07-0995a2151127
Attached file stack
[Tracking Requested - why for this release]: Shouldn't be shipping crash regressions
Flags: needinfo?(mats)
Attached file frame tree
The <optgroup> has a child nsBulletFrame which returns null from the GetContentInsertionFrame() call.
Assignee: nobody → mats
Flags: needinfo?(mats)
Attached patch fixSplinter Review
Perhaps this function should also skip nested <optgroup>s? (as bug 1228876)
Attachment #8694213 - Flags: review?(bzbarsky)
Attached patch crashtestSplinter Review
Comment on attachment 8694213 [details] [diff] [review]
fix

r=me, but please add a comment explaining why "frame" might be null here (if "option" is an anonymous leaf frame of some sort, sounds like).

Skip nested optgroups in what sense?  There should be no such thing in the frame tree; the frame constructor enforces that.
Attachment #8694213 - Flags: review?(bzbarsky) → review+
Tracked for 44 since it's a crash.
> There should be no such thing in the frame tree;

Ah, good point.  Thanks.
Flags: in-testsuite+
Comment on attachment 8694213 [details] [diff] [review]
fix

Approval Request Comment
[Feature/regressing bug #]: 1212688
[User impact if declined]: crash
[Describe test coverage new/current, TreeHerder]: have crashtest
[Risks and why]: zero risk, just an added null-pointer check
[String/UUID change made/needed]: none
Attachment #8694213 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/0d4f9aad0350
https://hg.mozilla.org/mozilla-central/rev/18149c916fbf
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Comment on attachment 8694213 [details] [diff] [review]
fix

Crash fixes are always good. Aurora44+
Attachment #8694213 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Jesse, could you please verify that the crash is gone? Thanks!
Flags: needinfo?(jruderman)
Yep, this is fixed for me on mozilla-central. The patch landed with a crashtest, btw :)
Flags: needinfo?(jruderman)
(In reply to Jesse Ruderman from comment #16)
> Yep, this is fixed for me on mozilla-central. The patch landed with a
> crashtest, btw :)

Thanks for the verification and crashtests are awesome :)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.