[webkit-reviews] review denied: [Bug 68149] CSS 2.1 failure: dynamic-top-change-001 to 004 fail : [Attachment 107668] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Robert Hogan <robert at webkit.org>'s
request for review:
Bug 68149: CSS 2.1 failure: dynamic-top-change-001 to 004 fail
https://bugs.webkit.org/show_bug.cgi?id=68149

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

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
Ah, nice to see someone tackling this old gem! It's trickier than it looks!

I like the idea of trying to track this in the RenderStyle. However, you're
patching the wrong place. I think you just want to make Node::diff return the
right value (Inherited) when an inherited property change is detected. I would
even go further and have RenderStyles maintain a hash of all properties they
inherit. If you know the actual properties, then the diff could actually check
to see only if those specific properties changed.

It seems like we should be able to do better as far as places we have to patch.
There should be a chokepoint we can find for inheritance. Just patching a few
places in CSSStyleApplyProperty.cpp doesn't cut it. Probably a good chokepoint
is when the declarations have been collected and are being walked.

I think you should be able to maintain some sort of map as you're applying the
declarations (that gets cleared as non-inherited values are encountered). Then
what you're left with is what you could put on the RenderStyle.