[webkit-reviews] review granted: [Bug 95117] [META] Move generated content into the DOM : [Attachment 167888] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Elliott Sprehn
<esprehn at chromium.org>'s request for review:
Bug 95117: [META] Move generated content into the DOM
https://bugs.webkit.org/show_bug.cgi?id=95117

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=167888&action=review


The only question is the perf. :)

> Source/WebCore/dom/Element.cpp:1820
> +	   existing->setNeedsStyleRecalc();
> +	   existing->recalcStyle(change);

Why do we need to setNeedsStyleRecalc()?  And do you need to upgrade the change
to Detach or Force?  It seems any style change could be a pseudo change, so we
do need this setNeedsStyleRecalc or we need to pass Force.  You should document
this now that we understand what's going on.

> Source/WebCore/dom/PseudoElement.cpp:107
> +    if (child->isText())
> +	   child->setStyle(renderStyle());
> +    else if (child->isImage()) {

Instead add an early return, which is if (!child->isText() ||
!child->isImage()) with a note, saying that it must not be our rendererer so we
don't manage the style.

Then ahve a "optimized" path for text.

And then fallback for images to the normal inherit path.

> Source/WebCore/dom/PseudoElement.cpp:122
> +    // PseudoElements are not part of the DOM tree and don't participte in
the recalcStyle flow so we must
> +    // propagate the style downward manually.

note to you: YOu said you wanted to update this.