[webkit-reviews] review denied: [Bug 122097] ASSERTION FAILED: !style->propertyIsImportant(propertyID) in WebCore::setTextDecorationProperty : [Attachment 214148] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 14 09:33:22 PDT 2013


Ryosuke Niwa <rniwa at webkit.org> has denied Santosh Mahto
<santosh.ma at samsung.com>'s request for review:
Bug 122097: ASSERTION FAILED: !style->propertyIsImportant(propertyID) in
WebCore::setTextDecorationProperty
https://bugs.webkit.org/show_bug.cgi?id=122097

Attachment 214148: Patch
https://bugs.webkit.org/attachment.cgi?id=214148&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=214148&action=review


> Source/WebCore/ChangeLog:25
> +	   (WebCore::StyleChange::extractTextStyles): Remove property even if 
> +	   it is declared !important.

Why?

> Source/WebCore/editing/EditingStyle.cpp:1446
> -	   setTextDecorationProperty(style, newTextDecoration.get(),
CSSPropertyTextDecoration);
> +	   if (newTextDecoration->length())
> +	       setTextDecorationProperty(style, newTextDecoration.get(),
CSSPropertyTextDecoration);
> +	   else
> +	       style->removeProperty(CSSPropertyTextDecoration);
> +

This doesn't make any sense. The whole point of setTextDecorationProperty is to
take care of that.
We should figure out why setTextDecorationProperty has the assertion instead of
working around it here.

> LayoutTests/editing/execCommand/remove-format-textdecoration-in-iframe.html:6

> +    <iframe onload=" {     document.designMode=&apos;on&apos;;
> +		      document.execCommand(&apos;selectall&apos;);
> +		      document.execCommand(&apos;RemoveFormat&apos;);	   } ">


Please fix the indentation.
Also, we should use dump-as-text or dump-as-markup.
There is no reason for this test to be a render tree dump.
r- because of this.


More information about the webkit-reviews mailing list