[Webkit-unassigned] [Bug 99295] removeAttribute('style') not working in certain circumstances

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=99295





--- Comment #10 from Ryosuke Niwa <rniwa at webkit.org>  2012-11-03 18:24:22 PST ---
(From update of attachment 169836)
View in context: https://bugs.webkit.org/attachment.cgi?id=169836&action=review

> Source/WebCore/ChangeLog:9
> +        style should be always removed by using "removeAttribute('style')".

Nit: s/removed by using/removable by/

> Source/WebCore/dom/StyledElement.cpp:224
> +void StyledElement::clearInlineStyleProperty()

Should be clearInlineStyleProperties or removeAllInlineStyleProperties.

> Source/WebCore/dom/StyledElement.cpp:228
> +    StylePropertySet* inlineStylePropertySet = ensureInlineStyle();

Why do you want to "ensure" it? We would have exited early if inline style was null.

> LayoutTests/fast/css/remove-attribute-style-expected.txt:6
> +PASS window.getComputedStyle(c11).backgroundColor is "rgba(0, 0, 0, 0)"

This output isn't great. I can't tell which line is testing what with cryptic names like c11.

> LayoutTests/fast/css/remove-attribute-style.html:19
> +      <td id="c11">no HTML style attribute, no get/setAttribute</td>

It would have been better if the id was elementWithoutStyleAttribute.

> LayoutTests/fast/css/remove-attribute-style.html:41
> +    document.body.offsetLeft;

Do we really need to trigger layout?

> LayoutTests/fast/css/remove-attribute-style.html:43
> +    shouldBe("window.getComputedStyle(c11).backgroundColor", '"rgba(0, 0, 0, 0)"');

You can omit "window."

> LayoutTests/fast/css/remove-attribute-style.html:62
> +    c22.getAttribute('style');

It would have been better if this line was included in shouldBe so that we can see what you're doing in the expected result.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.