[webkit-reviews] review granted: [Bug 99295] removeAttribute('style') not working in certain circumstances : [Attachment 172273] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Takashi Sakamoto
<tasak at google.com>'s request for review:
Bug 99295: removeAttribute('style') not working in certain circumstances
https://bugs.webkit.org/show_bug.cgi?id=99295

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

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


> Source/WebCore/dom/StyledElement.cpp:226
> +void StyledElement::clearInlineStyleProperties()

On my second thought, clearInlineStyleProperties sounds as if resetting values
of properties. So maybe we should call it removeAllInlineStyleProperties.

> LayoutTests/fast/css/remove-attribute-style.html:55
> +   
shouldBeNull("elementWithoutStyleAttributeWithGetAttrBeforeStyleUpdate.getAttri
bute('style')");
> +   
elementWithoutStyleAttributeWithGetAttrBeforeStyleUpdate.style.backgroundColor
= 'red';
> +   
elementWithoutStyleAttributeWithGetAttrBeforeStyleUpdate.removeAttribute('style
');
> +   
shouldBe("getComputedStyle(elementWithoutStyleAttributeWithGetAttrBeforeStyleUp
date).backgroundColor", '"rgba(0, 0, 0, 0)"');

You can do something like:
shouldBeNull("elementWithoutStyleAttribute2.getAttribute('style')");
shouldBe("elementWithoutStyleAttribute2.style.backgroundColor = 'red';
elementWithoutStyleAttribute2.removeAttribute('style');
elementWithoutStyleAttribute2.backgroundColor", "rgba(0, 0, 0, 0)");
so that the actual operations done before accessing backgroundColor is visible
in the results instead of giving a really long variable name.

> LayoutTests/fast/css/remove-attribute-style.html:65
> +    elementWithoutStyleAttributeWithSetAttr.style.backgroundColor = 'red';
> +    elementWithoutStyleAttributeWithSetAttr.setAttribute('style', '');
> +    elementWithoutStyleAttributeWithSetAttr.removeAttribute('style');
> +   
shouldBe("getComputedStyle(elementWithoutStyleAttributeWithSetAttr).backgroundC
olor", '"rgba(0, 0, 0, 0)"');

Similarly here, you can do:
shouldBe("elementWithoutStyleAttribute3.style.backgroundColor = 'red';
elementWithoutStyleAttribute3.setAttribute('style', '');
elementWithoutStyleAttribute3.removeAttribute('style);
getComputedStyle(elementWithoutStyleAttribute3).backgroundColor", '"rgba(0, 0,
0, 0)"');