[webkit-reviews] review denied: [Bug 55255] background-color misbehaving for :link versus :visited : [Attachment 103330] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Oct 23 06:31:28 PDT 2011


Antti Koivisto <koivisto at iki.fi> has denied Kentaro Hara <haraken at google.com>'s
request for review:
Bug 55255: background-color misbehaving for :link versus :visited
https://bugs.webkit.org/show_bug.cgi?id=55255

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

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


r- for bad test results. You will also need to explain why this is a
progression.

>> LayoutTests/fast/history/visited-link-default-background-color.html:17
>> +<p><a href="resources/dummy.html">This text should be on a white
background.</a></p>
> 
> If you just need it to match :visited, you can use <a href=""> which is much
simpler.

Use <a href="">, that is guaranteed to match visited on all platforms.

>
LayoutTests/platform/mac/fast/history/visited-link-default-background-color-exp
ected.txt:13
> +	   RenderInline {A} at (0,0) size 273x18 [color=#0000EE]
[bgcolor=#808000]
> +	     RenderText {#text} at (0,0) size 273x18
> +	       text run at (0,0) width 273: "This text should be on a white
background."

This test result is wrong. The background is not white.

> Source/WebCore/rendering/style/RenderStyle.cpp:-1192
> -    // FIXME: Technically someone could explicitly specify the color
transparent, but for now we'll just
> -    // assume that if the background color is transparent that it wasn't
set. Note that it's weird that
> -    // we're returning unvisited info for a visited link, but given our
restriction that the alpha values
> -    // have to match, it makes more sense to return the unvisited background
color if specified than it
> -    // does to return black. This behavior matches what Firefox 4 does as
well.

Why is this a good change? The comment says that the current behavior matches
Firefox.


More information about the webkit-reviews mailing list