[webkit-reviews] review denied: [Bug 6503] content property doesn't support quotes : [Attachment 81638] Proposed Patch. Similar to Darin's, but taking advantage of the recent changes to RenderObject

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Feb 8 10:14:47 PST 2011


Dave Hyatt <hyatt at apple.com> has denied Carol Szabo <carol.szabo at nokia.com>'s
request for review:
Bug 6503: content property doesn't support quotes
https://bugs.webkit.org/show_bug.cgi?id=6503

Attachment 81638: Proposed Patch. Similar to Darin's, but taking advantage of
the recent changes to RenderObject
https://bugs.webkit.org/attachment.cgi?id=81638&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
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.


More information about the webkit-reviews mailing list