Bug 58233 - fast/css/dashboard-regions-attr-crash.html asserts
Summary: fast/css/dashboard-regions-attr-crash.html asserts
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 53488
  Show dependency tree / graph
 
Reported: 2011-04-11 07:19 PDT by Csaba Osztrogonác
Modified: 2011-04-12 09:00 PDT (History)
2 users (show)

See Also:

Attachments
proposed fix (2.10 KB, patch)
2011-04-12 08:18 PDT, Csaba Osztrogonác 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2011-04-11 07:19:05 PDT 
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)
Comment 1 Andras Becsi 2011-04-11 08:48:10 PDT 
(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.
Comment 2 Csaba Osztrogonác 2011-04-12 08:18:18 PDT 
Created attachment 89205 [details]
proposed fix
Comment 3 Andras Becsi 2011-04-12 08:30:20 PDT 
(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.
Comment 4 Csaba Osztrogonác 2011-04-12 08:42:25 PDT 
(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)
Comment 5 Benjamin Poulain 2011-04-12 08:55:00 PDT 
Comment on attachment 89205 [details]
proposed fix

Look sane, trust you
Comment 6 Csaba Osztrogonác 2011-04-12 08:59:53 PDT 
Comment on attachment 89205 [details]
proposed fix

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