841225 - Crash [@ nsProgressFrame::ShouldUseNativeStyle] with svg.text.css-frames.enabled

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jesse Ruderman]

Attachment: testcase

With:
  user_pref("svg.text.css-frames.enabled", true);

Crash [@ nsProgressFrame::ShouldUseNativeStyle]

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Daniel Holbert [:dholbert]]

mBarDiv->GetPrimaryFrame() is null, so we're crashing on this line:

> 290 mBarDiv->GetPrimaryFrame()->GetStyleDisplay()->mAppearance == NS_THEME_PROGRESSBAR_CHUNK &&
https://mxr.mozilla.org/mozilla-central/source/layout/forms/nsProgressFrame.cpp#290

Comment 3

[Cameron McCormack (:heycam)]

I don't think we should be creating a frame for the <progress> element inside the SVG <text> element.

Comment 4

[Cameron McCormack (:heycam)]

Although we already have code to prevent this https://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSFrameConstructor.cpp#5306 so maybe that's working in this case.

Comment 5

[Cameron McCormack (:heycam)]

*not working

Comment 6

[Cameron McCormack (:heycam)]

Attachment: patch

Inside a <tspan>, we're creating frame construction items without a parent frame available to check.  So we should be checking the ITEM_IS_SVG_TEXT flag too, to avoid creating frames for non-SVG elements inside SVG text.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 713782 [details] [diff] [review]
patch

>+          (aFlags & ITEM_IS_WITHIN_SVG_TEXT))) {

This is mis-indented, leading to confusion.  Please fix.

r=me with that.

Comment 8

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/86d7b194cd33

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/86d7b194cd33