RESOLVED FIXED 54464
Rename Color::name() to Color::nameForRenderTreeAsText()
https://bugs.webkit.org/show_bug.cgi?id=54464
Summary Rename Color::name() to Color::nameForRenderTreeAsText()
Andreas Kling
Reported 2011-02-15 08:24:42 PST
Color::name() returns the color as either #RRGGBB or #RRGGBBAA. Since the latter is not a valid CSS color, it can't be re-parsed by WebKit, and should only be used in DRT dumps. Existing call sites that wrongly use drtName() will be updated separately with accompanying layout tests.
Attachments
Proposed patch (11.75 KB, patch)
2011-02-15 08:26 PST, Andreas Kling
darin: review+
Andreas Kling
Comment 1 2011-02-15 08:26:24 PST
Created attachment 82460 [details] Proposed patch
mitz
Comment 2 2011-02-15 08:35:05 PST
drtName us quite cryptic, and the function is not called by DumpRenderTree anyway. How about nameForRenderTreeAsText(), externalRepresentation(), or nameForExternalRepresentation()?
Andreas Kling
Comment 3 2011-02-15 08:38:34 PST
(In reply to comment #2) > drtName us quite cryptic, and the function is not called by DumpRenderTree anyway. How about nameForRenderTreeAsText(), externalRepresentation(), or nameForExternalRepresentation()? I'm not fussed about the name- nameForRenderTreeAsText() sounds fine to me.
Darin Adler
Comment 4 2011-02-15 10:07:11 PST
Comment on attachment 82460 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=82460&action=review Please use a better name, though! > Source/WebCore/platform/graphics/Color.h:101 > + // Returns the color serialized as either #RRGGBB or #RRGGBBAA > + // The latter format is not a valid CSS color, and should only be seen in DRT dumps. > + String drtName() const; I suggest nameForDumpRenderTree or nameForRenderTreeDumps, or nameForRegressionTestDumps or nameForLayoutTests or something along those lines. Not a big fan of acronyms, and they are even worse as all lowercase prefixes!
Darin Adler
Comment 5 2011-02-15 10:07:57 PST
(In reply to comment #3) > (In reply to comment #2) > > drtName us quite cryptic, and the function is not called by DumpRenderTree anyway. How about nameForRenderTreeAsText(), externalRepresentation(), or nameForExternalRepresentation()? > > I'm not fussed about the name- nameForRenderTreeAsText() sounds fine to me. I like that one too. Mitz’s names are all better than mine. I didn’t see Mitz’s comments before I posted mine.
Andreas Kling
Comment 6 2011-02-15 11:02:45 PST
Note You need to log in before you can comment on or make changes to this bug.