[webkit-reviews] review requested: [Bug 6831] contentEditable outline darkens as caret moves : [Attachment 6583] patch 3

[bugzilla-request-daemon at opendarwin.org]

Graham Dennis <Graham.Dennis at gmail.com> has asked Darin Adler <darin at apple.com>
for review:
Bug 6831: contentEditable outline darkens as caret moves
http://bugzilla.opendarwin.org/show_bug.cgi?id=6831

Attachment 6583: patch 3
http://bugzilla.opendarwin.org/attachment.cgi?id=6583&action=edit

------- Additional Comments from Graham Dennis <Graham.Dennis at gmail.com>
A new patch addressing Darin's comments.

Darin: I didn't understand exactly why WebClipView was there, but thanks for
explaining it. But _focusRingVisibleRect is not just called by AppKit, it's
also called by WKSetFocusRingStyle (in WebKitSystemInterface). When drawing the
focus ring, the clip rects aren't respected as the focus ring typically goes
outside the element that is being drawn, and so it doesn't obey it's clip
rects. As a result, the nice simple solution of NSRectClipList doesn't work (I
had tried this previously). I also tried your suggestion of
CGContextClipToRects, but that didn't work either. I'll attach a patch that
adds in the code to call this, just so that you can check that I didn't make a
mistake when I tried it, and that it does in fact not work. The way to clip the
focus ring when it is being drawn is through the style that is set in
WKSetFocusRingStyle (or alternatively in WKDrawFocusRing).

So, in this version of the patch, I have removed all reference to the clip view
from WKDrawFocusRing so that it can in fact go into WebKitSystemInterface. The
rects being drawn are found directly from the focussed NSView. The reason we
need to iterate over [focusRingPath fill] is that the rectangles that we want
to draw in will not necessarily be contiguous.

Also, in the line 'focusRingDescriptor.radius = ...' in WKDrawFocusRing, I
think the (float)2.0 may in fact be a global variable, but I can't know its
name without the actual source to WKSetFocusRingStyle. Likewise, all of the
structure member names will be wrong, and will need to be corrected.

I'll also attach an automatic testcase (it can only be tested by looking at the
pixels).