Created attachment 172097 [details] Asserts on debug builds. Looks like we're not handling CSSPropertyColorProfile in StyleResolver::applySVGProperty(): STDERR: ASSERTION FAILED: unimplemented propertyID: 1357 STDERR: 0 STDERR: ../../third_party/WebKit/Source/WebCore/css/SVGCSSStyleSelector.cpp(596) : void WebCore::StyleResolver::applySVGProperty(WebCore::CSSPropertyID, WebCore::CSSValue*) Chromium issue: http://code.google.com/p/chromium/issues/detail?id=157492
Created attachment 172101 [details] Patch
Attachment 172101 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/rendering/style/SVGRenderStyle.h:409: _colorProfile is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #2) > Source/WebCore/rendering/style/SVGRenderStyle.h:409: _colorProfile is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Just following the local style there.
Comment on attachment 172101 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=172101&action=review Since we don't handle it at all, why not do the same like for background-enable: case CSSPropertyEnableBackground: // Silently ignoring this property for now Some might use at as feature detection. I would rather not enable it in CSSOM, when we don't support it at all. >> Source/WebCore/rendering/style/SVGRenderStyle.h:409 >> + unsigned _colorProfile : 1; // EColorProfile > > _colorProfile is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Well, it should still be fixed IMHO. Maybe in a preparation patch?
(In reply to comment #4) > (From update of attachment 172101 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=172101&action=review > > Since we don't handle it at all, why not do the same like for background-enable: > > case CSSPropertyEnableBackground: > // Silently ignoring this property for now Since I thought we'll eventually add color-profile support, I figured it's best to fix the crash by building the required plumbing. > Some might use at as feature detection. I would rather not enable it in CSSOM, when we don't support it at all. Ah, that makes sense. I'll update the patch. > >> Source/WebCore/rendering/style/SVGRenderStyle.h:409 > >> + unsigned _colorProfile : 1; // EColorProfile > > > > _colorProfile is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] > > Well, it should still be fixed IMHO. Maybe in a preparation patch? Sure, that can be fixed. But we'll be ignoring CSSPropertyColorProfile for now, so it's completely unrelated.
Created attachment 172223 [details] Patch
Comment on attachment 172223 [details] Patch Clearing flags on attachment: 172223 Committed r133420: <http://trac.webkit.org/changeset/133420>
All reviewed patches have been landed. Closing bug.