[webkit-reviews] review requested: [Bug 9197] CSS3: Borders with border-radius and double, groove, or ridge styles should look better : [Attachment 59390] Updated, re-factored version of Alex's patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 22 10:41:19 PDT 2010


Beth Dakin <bdakin at apple.com> has asked  for review:
Bug 9197: CSS3: Borders with border-radius and double, groove, or ridge styles
should look better
https://bugs.webkit.org/show_bug.cgi?id=9197

Attachment 59390: Updated, re-factored version of Alex's patch
https://bugs.webkit.org/attachment.cgi?id=59390&action=review

------- Additional Comments from Beth Dakin <bdakin at apple.com>
Here is an updated, re-factored, and ever-so-slightly behaviorally different
version of Alex's patch. This is probably best reviewed in person where I can
show all of the improvements and the trade-offs, but I will try to summarize
here in case anyone who is not right HERE wants to review:

-Solid, inset/outset, groove/ridge border are WAY better. I am tempted to say
this patch fixes all of the bugs with these styles based on Dan's interactive
test case, but there might be some edge cases.
-Double border are way better too. There are a few edge-case bugs that can be
found with the interactive test case, but I think these could be fine-tuned and
fixed in a follow-up patch.
-Dotted and Dashed borders are not better, but I am tempted to say they are not
worse either. They are just…different. This is the area that would need the
most follow-up work if we decide to go with this patch. These layout tests are
probably worse with this patch:

fast/borders/borderRadiusDashed01.html
fast/borders/borderRadiusDashed02.html
fast/borders/borderRadiusDashed03.html

But these ones are better:

fast/borders/borderRadiusDotted01.html
fast/borders/borderRadiusDotted02.html
fast/borders/borderRadiusDotted03.html

This one is a good example of "just different." I don't think the new rendering
is better than the old or vice versa.

fast/borders/borderRadiusAllStylesAllCorners.html

Playing with the interactive test case with and without the patch make me feel
like it's just trading one set of bugs for another. 

Other obvious problems with this patch are that I need to implement
clipConvexPolygon() for non-CG platforms, and need to add many news tests. I
will work on that while all of you reviewers out there consider taking a look
;-)


More information about the webkit-reviews mailing list