[webkit-reviews] review granted: [Bug 71703] REGRESSION(r98542): Chromium: CSS text is rendered on page : [Attachment 115934] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Nov 19 04:41:22 PST 2011


Antti Koivisto <koivisto at iki.fi> has granted David Barr
<davidbarr at chromium.org>'s request for review:
Bug 71703: REGRESSION(r98542): Chromium: CSS text is rendered on page
https://bugs.webkit.org/show_bug.cgi?id=71703

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

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=115934&action=review


r=me, please keep an eye on that the test actually passes on all platforms when
landing.

> Source/WebCore/css/CSSStyleSelector.cpp:2285
>  void CSSStyleSelector::applyMatchedDeclarations(const MatchResult&
matchResult)
>  {
> -    unsigned cacheHash = matchResult.isCacheable ?
computeDeclarationHash(m_matchedDecls.data(), m_matchedDecls.size()) : 0;
> +    unsigned cacheHash = !simpleDefaultStyleSheet && matchResult.isCacheable
? computeDeclarationHash(m_matchedDecls.data(), m_matchedDecls.size()) : 0;

The ternary statement is getting bit ugly. You could just set isCacheable to
false beforehand, that reads better too.

> LayoutTests/fast/css/style-tag-display-none-expected.txt:8
> +layer at (0,0) size 800x600
> +  RenderView at (0,0) size 800x600
> +layer at (0,0) size 800x8
> +  RenderBlock {HTML} at (0,0) size 800x8
> +    RenderBody {BODY} at (8,8) size 784x0
> +	 RenderInline {A} at (0,0) size 0x0 [color=#551A8B]
> +	 RenderText {#text} at (0,0) size 0x0
> +	 RenderText {#text} at (0,0) size 0x0

Not sure if this render tree dump is actually valid on all platforms.


More information about the webkit-reviews mailing list