[webkit-reviews] review granted: [Bug 69838] Resolve regular and visited link style in a single pass : [Attachment 110545] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 11 17:33:47 PDT 2011


Darin Adler <darin at apple.com> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 69838: Resolve regular and visited link style in a single pass
https://bugs.webkit.org/show_bug.cgi?id=69838

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=110545&action=review


I read over this and did not spot any mistakes.

> Source/WebCore/css/CSSStyleSelector.cpp:210
> -    unsigned m_position : 28;
> +    unsigned m_position : 26;

How many bits do we really need for this? Can it be even less than 26? How do
we handle overflow?

> Source/WebCore/css/CSSStyleSelector.cpp:1292
> +	   // pseudo element styles will continue to work for pseudo elements
inside :visited
> +	   // links.

I’d put that last word on the previous line.

> Source/WebCore/css/SelectorChecker.h:90
> +    enum LinkMatchMask {  MatchLink = 1, MatchVisited = 2, MatchAll =
MatchLink | MatchVisited };

Extra space here before MatchLink.

> Source/WebCore/rendering/style/RenderStyle.cpp:260
> +    ASSERT(pseudo->styleType() > NOPSEUDO);

Might be worth a “why this is a good assertion” comment.


More information about the webkit-reviews mailing list