Bug 58233 – fast/css/dashboard-regions-attr-crash.html asserts

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Csaba Osztrogonác]

fast/css/dashboard-regions-attr-crash.html asserts on the Qt debug bots.
It is is a very very old assertion.

stdout for r83435:
-------------------
ASSERTION FAILED: unimplemented propertyID: 1287
../../../Source/WebCore/css/SVGCSSParser.cpp(294) : bool WebCore::CSSParser::parseSVGValue(int, bool)

[Andras Becsi]

(In reply to comment #0)
> fast/css/dashboard-regions-attr-crash.html asserts on the Qt debug bots.
> It is is a very very old assertion.
> 
> stdout for r83435:
> -------------------
> ASSERTION FAILED: unimplemented propertyID: 1287
> ../../../Source/WebCore/css/SVGCSSParser.cpp(294) : bool WebCore::CSSParser::parseSVGValue(int, bool)

AFAIK dashboard region is Mac-specific, and in CSSParser::parseValue the CSSPropertyWebkitDashboardRegion case is guarded by ENABLE(DASHBOARD_SUPPORT). Since Qt does not support it, it falls though further to parseSVGValue where in the default case there is this ASSERT_WITH_MESSAGE.

I think the test should either be skipped permanently or there should be an elif define with a break in parseValue to catch the CSSPropertyWebkitDashboardRegion property.

I vote for the first approach ot make the debug bot happy.

[Csaba Osztrogonác]

Created attachment 89205 [details]
proposed fix

[Andras Becsi]

(In reply to comment #2)
> Created an attachment (id=89205) [details]
> proposed fix

LGTM!

Even better!

For features.pri having -= lines is simply wrong, maybe even check-webkit-style could shout for it.

[Csaba Osztrogonác]

(In reply to comment #3)
> (In reply to comment #2)
> > Created an attachment (id=89205) [details] [details]
> > proposed fix
> 
> LGTM!
> 
> Even better!
> 
> For features.pri having -= lines is simply wrong, maybe even check-webkit-style could shout for it.

It was good long time ago, because the initialization of DASHBOARDSUPPORTCSSPROPERTIES was before this -= hack.
(When there aren't any WebCore.pri, DerivedSources.pro, 
CodeGenerators.pri, but a monumental and ugly WebCore.pro)

[Benjamin Poulain]

Comment on attachment 89205 [details]
proposed fix

Look sane, trust you

[Csaba Osztrogonác]

Comment on attachment 89205 [details]
proposed fix

Landed in http://trac.webkit.org/changeset/83593