[webkit-reviews] review denied: [Bug 106535] getComputedStyle() doesn't report intermediate values during a transition of a pseudo element : [Attachment 182873] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has denied Elliott Sprehn
<esprehn at chromium.org>'s request for review:
Bug 106535: getComputedStyle() doesn't report intermediate values during a
transition of a pseudo element
https://bugs.webkit.org/show_bug.cgi?id=106535

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

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=182873&action=review


Fine at a high-level. Just a few nits.

Mainly R- because I'm hoping we can tighten up the tests.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1509
> +    Node* styledNode = this->styledNode();

Nit: Can we make this styledElement? You can't get computed style on a
non-Element anymore with Antti's recent refactorings.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1513
> +    Document* document = styledNode->document();

Do you need to move this? pseudo elements are necessarily in the same document
as their node, no?

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1550
> +	   style = m_node->computedStyle(m_pseudoElementSpecifier);

May as well use styledNode and avoid the need for the comment?

> LayoutTests/fast/css-generated-content/pseudo-animation.html:86
> +	   shouldBeCloseTo('div.offsetWidth', 20, 1);

Why shouldBeCloseTo instead of shouldBe? Seems like we should be able to modify
the test so that we can get a hard value.