[Webkit-unassigned] [Bug 64861] Need support for :dir() pseudo-class

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=64861

--- Comment #8 from Benjamin Poulain <benjamin at webkit.org> ---
Comment on attachment 248025
  --> https://bugs.webkit.org/attachment.cgi?id=248025
Work in progress

View in context: https://bugs.webkit.org/attachment.cgi?id=248025&action=review

Quick review

> Source/WebCore/css/CSSSelector.cpp:339
>  bool CSSSelector::operator==(const CSSSelector& other) const

Missing blank line between the methods.

> Source/WebCore/css/SelectorChecker.cpp:58
> + #include "TextDirection.h"

Extra space before the #

> Source/WebCore/css/SelectorChecker.cpp:1012
>          // FIXME: Implement :dir() selector.

Don't forget to remove the comment. :)

> Source/WebCore/css/SelectorChecker.cpp:1014
> +            {

This bracket should be on the previous line.

> Source/WebCore/css/SelectorChecker.cpp:1018
> +                if (!CSSSelector::parseDirection(selector->argument(), dir)) {
> +                    return false;
> +                }

The WebKit coding style is against brackets for single conditional statements.

> Source/WebCore/css/SelectorChecker.cpp:1019
> +                return (element->computeInheritedDirection() == dir);

No need for the parenthesis here.

> Source/WebCore/dom/Element.cpp:2424
> +    return LTR; // Must ASSERT we don't get here

ASSERT_NOT_REACHED()

> Source/WebCore/dom/Element.h:384
> +    enum DirAttributeState { DirLTR, DirRTL, DirAuto, DirUnknown };
> +    DirAttributeState dir() const;
> +    TextDirection computeInheritedDirection() const;

For the enum, you can use a enum class.

Shouldn't dir() be private?

Maybe it is personal taste, but I am not a big fan of the abbreviated "dir".

Maybe?:
    enum class SelfDirectionality { LTR, RTL, Auto, Unknown }
    selfDirectionality()
    computeInheritedDirectionality();

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150306/3b644b07/attachment-0002.html>