[webkit-reviews] review denied: [Bug 21680] queryCommandValue("BackColor") returns rgb(0, 0, 0) for elements with transparent background : [Attachment 61094] fixed style: merged valueStyle in EditorCommand.cpp and Frame::selectionStartStylePropertyValue and added a special case to treat background color

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 21680: queryCommandValue("BackColor") returns rgb(0,0,0) for elements with
transparent background
https://bugs.webkit.org/show_bug.cgi?id=21680

Attachment 61094: fixed style: merged valueStyle in EditorCommand.cpp and
Frame::selectionStartStylePropertyValue and added a special case to treat
background color
https://bugs.webkit.org/attachment.cgi?id=61094&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
The special case for background color, climbing the tree and finding
backgrounds on other elements, seems strange to me and does not seem like the
right way to go. Do other browsers have behavior like this?

Instead of "value->getRGBA32Value() == makeRGBA(0, 0, 0, 0)" you should just
check for an alpha of 0.

Getting selectionStartStylePropertyValue out of the Frame class is a good idea.
Putting all this logic into EditorCommand.cpp is not so good. The point of that
file is to be a dispatch point for the various editing commands. Any command
that has a nontrivial algorithm should be contained in functions in other
files. This algorithm in particular is complex and I'm not sure that having it
here is good.