Bug 73703 - StyledElement: Simplify addCSSColor().
Summary: StyledElement: Simplify addCSSColor().
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-02 14:18 PST by Andreas Kling
Modified: 2011-12-02 15:48 PST (History)
0 users

See Also:


Attachments
Patch (2.75 KB, patch)
2011-12-02 14:19 PST, Andreas Kling
darin: review+
kling: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2011-12-02 14:18:00 PST
StyledElement: Simplify addCSSColor().
Comment 1 Andreas Kling 2011-12-02 14:19:06 PST
Created attachment 117689 [details]
Patch
Comment 2 Darin Adler 2011-12-02 14:32:01 PST
Comment on attachment 117689 [details]
Patch

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

> Source/WebCore/dom/StyledElement.cpp:374
> +    // An empty string doesn't apply a color. (Only whitespace does, which is why this check occurs before trimming.)

This wording (and the old wording) is terribly confusing.

The phrase “only whitespace applies a color” is not what we mean here. What we mean is “A string containing only whitespace applies a color”. So we need to word it that way: “A string containing only”, “One containing only”, “A string with only”, or “One with only”.

The term “trimming” is confusing, since the function called below is stripWhiteSpace, and does not use the word “trim”.

> Source/WebCore/dom/StyledElement.cpp:388
> +    Color color(colorString);

I think I’d call this local variable parsedColor.
Comment 3 Andreas Kling 2011-12-02 15:48:55 PST
Committed r101870: <http://trac.webkit.org/changeset/101870>