[Webkit-unassigned] [Bug 6503] content property doesn't support quotes

[bugzilla-daemon at webkit.org]

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


Dave Hyatt <hyatt at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #81638|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #21 from Dave Hyatt <hyatt at apple.com>  2011-02-08 10:14:48 PST ---
(From update of attachment 81638)
View in context: https://bugs.webkit.org/attachment.cgi?id=81638&action=review

Glad you are tackling this!  Some initial comments:

> Source/WebCore/css/CSSStyleSelector.cpp:4599
> +        HANDLE_INHERIT(quotes, Quotes)

Where is the handling of "initial"?

> Source/WebCore/css/html.css:28
> +    quotes: '"' '"' "'" "'";

This really shouldn't be necessary.  The CSS 2.1 specification says the initial value "depends on the user agent," which implies to me that the initial value is the quotes you want to use and not none.  Also, quotes should obviously work for XML documents, so let's just fix the initial value.

The initial value should probably just be a special keyword like -webkit-quotes.  Then your front end code can eventually use the current document language to pick the correct set of quotes (instead of hard-coding the quotes listed above).

> Source/WebCore/rendering/RenderQuote.cpp:80
> +    for (RenderObject* predecesor = head->previousInPreOrder(); predecesor; predecesor = predecesor->previousInPreOrder())

Should be predecessor.

> Source/WebCore/rendering/RenderQuote.cpp:178
> +        if (descendant->isQuote()) {

I think this for loop would read nicer if you just did

if (!descendant->isQuote())
    continue;

and then you don't have to indent all the code that comes after.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.