2010-10-14 Daniel Bates <dbates@rim.com>
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Oct 2010 15:40:46 +0000 (15:40 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Oct 2010 15:40:46 +0000 (15:40 +0000)
        Reviewed by Darin Adler.

        Only draw focus ring in RenderInline and RenderImage if the theme
        is not able to draw a focus ring
        https://bugs.webkit.org/show_bug.cgi?id=47632

        Fixes an issue where RenderInline::paintOutline() and RenderImage::paintFocusRings()
        would draw a focus ring regardless of whether the port-specific theme is able to
        draw a focus ring. Instead, these methods should only draw a focus ring if the
        theme is unable to draw a focus ring.

        Also, extracted common focus ring drawing code from RenderObject::paintOutline()
        and RenderInline::paintOutline() into RenderObject::paintFocusRing().

        Tests: fast/forms/textfield-focus-ring.html
               fast/images/imagemap-focus-ring.html
               fast/inline/inline-focus-ring.html

        * rendering/RenderImage.cpp:
        (WebCore::RenderImage::paintFocusRings): Modified to only draw a focus ring if the
        the theme does not draw one.
        * rendering/RenderInline.cpp:
        (WebCore::RenderInline::paintOutline): Modified to call RenderObject::paintFocusRing().
        * rendering/RenderObject.cpp:
        * rendering/RenderObject.cpp:
        (WebCore::RenderObject::paintFocusRing): Added.
        (WebCore::RenderObject::paintOutline): Modified to call RenderObject::paintFocusRing().
        * rendering/RenderObject.h:
2010-10-14  Daniel Bates  <dbates@rim.com>

        Reviewed by Darin Adler.

        Only draw focus ring in RenderInline and RenderImage if the theme
        is not able to draw a focus ring
        https://bugs.webkit.org/show_bug.cgi?id=47632

        Pixel tests to ensure that we don't regress focus ring drawing for RenderObject
        (fast/forms/textfield-focus-ring.html), RenderImage (fast/images/imagemap-focus-ring.html)
        and RenderInline (fast/inline/inline-focus-ring.html) on ports that support focus
        ring drawing.

        * fast/forms/textfield-focus-ring.html: Added.
        * fast/images/imagemap-focus-ring.html: Added.
        * fast/inline/inline-focus-ring.html: Added.
        * platform/mac/fast/forms/textfield-focus-ring-expected.checksum: Added.
        * platform/mac/fast/forms/textfield-focus-ring-expected.png: Added.
        * platform/mac/fast/forms/textfield-focus-ring-expected.txt: Added.
        * platform/mac/fast/images/imagemap-focus-ring-expected.checksum: Added.
        * platform/mac/fast/images/imagemap-focus-ring-expected.png: Added.
        * platform/mac/fast/images/imagemap-focus-ring-expected.txt: Added.
        * platform/mac/fast/inline/inline-focus-ring-expected.checksum: Added.
        * platform/mac/fast/inline/inline-focus-ring-expected.png: Added.
        * platform/mac/fast/inline/inline-focus-ring-expected.txt: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@69766 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/forms/textfield-focus-ring.html [new file with mode: 0644]
LayoutTests/fast/images/imagemap-focus-ring.html [new file with mode: 0644]
LayoutTests/fast/inline/inline-focus-ring.html [new file with mode: 0644]
LayoutTests/platform/mac/fast/forms/textfield-focus-ring-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/fast/forms/textfield-focus-ring-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/fast/forms/textfield-focus-ring-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/fast/images/imagemap-focus-ring-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/fast/images/imagemap-focus-ring-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/fast/images/imagemap-focus-ring-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/fast/inline/inline-focus-ring-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/fast/inline/inline-focus-ring-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/fast/inline/inline-focus-ring-expected.txt [new file with mode: 0644]
WebCore/ChangeLog
WebCore/rendering/RenderImage.cpp
WebCore/rendering/RenderInline.cpp
WebCore/rendering/RenderObject.cpp
WebCore/rendering/RenderObject.h

index 39e73c2..5b08054 100644 (file)
@@ -1,3 +1,29 @@
+2010-10-14  Daniel Bates  <dbates@rim.com>
+
+        Reviewed by Darin Adler.
+
+        Only draw focus ring in RenderInline and RenderImage if the theme
+        is not able to draw a focus ring
+        https://bugs.webkit.org/show_bug.cgi?id=47632
+
+        Pixel tests to ensure that we don't regress focus ring drawing for RenderObject
+        (fast/forms/textfield-focus-ring.html), RenderImage (fast/images/imagemap-focus-ring.html)
+        and RenderInline (fast/inline/inline-focus-ring.html) on ports that support focus
+        ring drawing.
+
+        * fast/forms/textfield-focus-ring.html: Added.
+        * fast/images/imagemap-focus-ring.html: Added.
+        * fast/inline/inline-focus-ring.html: Added.
+        * platform/mac/fast/forms/textfield-focus-ring-expected.checksum: Added.
+        * platform/mac/fast/forms/textfield-focus-ring-expected.png: Added.
+        * platform/mac/fast/forms/textfield-focus-ring-expected.txt: Added.
+        * platform/mac/fast/images/imagemap-focus-ring-expected.checksum: Added.
+        * platform/mac/fast/images/imagemap-focus-ring-expected.png: Added.
+        * platform/mac/fast/images/imagemap-focus-ring-expected.txt: Added.
+        * platform/mac/fast/inline/inline-focus-ring-expected.checksum: Added.
+        * platform/mac/fast/inline/inline-focus-ring-expected.png: Added.
+        * platform/mac/fast/inline/inline-focus-ring-expected.txt: Added.
+
 2010-10-14  Stephen White  <senorblanco@chromium.org>
 
         Unreviewed; test expectations update.
diff --git a/LayoutTests/fast/forms/textfield-focus-ring.html b/LayoutTests/fast/forms/textfield-focus-ring.html
new file mode 100644 (file)
index 0000000..ada5cca
--- /dev/null
@@ -0,0 +1,15 @@
+<html>
+<head>
+<script>
+window.onload = function()
+{
+    document.getElementById("input").focus();
+}
+</script>
+</head>
+<body>
+<p>Assuming the port-specific theme draws focus rings, this test can be used to ensure that a focus ring is drawn for a text input element. This test PASSED if a focus ring is drawn around the text input element (below).</p>
+<input id="input" type="text"/>
+</body>
+</head>
+</html>
diff --git a/LayoutTests/fast/images/imagemap-focus-ring.html b/LayoutTests/fast/images/imagemap-focus-ring.html
new file mode 100644 (file)
index 0000000..5759cc4
--- /dev/null
@@ -0,0 +1,18 @@
+<html>
+<head>
+<script>
+window.onload = function()
+{
+    document.getElementById("area").focus();
+}
+</script>
+</head>
+<body>
+<p>Assuming the port-specific theme draws focus rings, this test can be used to ensure that a focus ring is drawn for an imagemap. This test PASSED if a focus ring is drawn around the &lt;area&gt; in the imagemap (below).</p>
+<map name="imagemap">
+    <area id="area" shape="rect" coords="0,0,128,128" href="#dummy" />
+</map>
+<img src="imagemap.jpg" width="128" height="128" usemap="#imagemap" ismap />
+</body>
+</head>
+</html>
diff --git a/LayoutTests/fast/inline/inline-focus-ring.html b/LayoutTests/fast/inline/inline-focus-ring.html
new file mode 100644 (file)
index 0000000..6970b4b
--- /dev/null
@@ -0,0 +1,15 @@
+<html>
+<head>
+<script>
+window.onload = function()
+{
+    document.getElementById("link").focus();
+}
+</script>
+</head>
+<body>
+<p>Assuming the port-specific theme draws focus rings, this test can be used to ensure that a focus ring is drawn for a hyperlink. This test PASSED if a focus ring is drawn around the hyperlink (below).</p>
+<a id="link" href="#" contentEditable="true">Hyperlink</a>
+</body>
+</head>
+</html>
diff --git a/LayoutTests/platform/mac/fast/forms/textfield-focus-ring-expected.checksum b/LayoutTests/platform/mac/fast/forms/textfield-focus-ring-expected.checksum
new file mode 100644 (file)
index 0000000..e655213
--- /dev/null
@@ -0,0 +1 @@
+9ad66b468518000eb8b6f73d1204bdb7
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/fast/forms/textfield-focus-ring-expected.png b/LayoutTests/platform/mac/fast/forms/textfield-focus-ring-expected.png
new file mode 100644 (file)
index 0000000..1a11264
Binary files /dev/null and b/LayoutTests/platform/mac/fast/forms/textfield-focus-ring-expected.png differ
diff --git a/LayoutTests/platform/mac/fast/forms/textfield-focus-ring-expected.txt b/LayoutTests/platform/mac/fast/forms/textfield-focus-ring-expected.txt
new file mode 100644 (file)
index 0000000..2431e67
--- /dev/null
@@ -0,0 +1,15 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 779x36
+          text run at (0,0) width 779: "Assuming the port-specific theme draws focus rings, this test can be used to ensure that a focus ring is drawn for a text input"
+          text run at (0,18) width 568: "element. This test PASSED if a focus ring is drawn around the text input element (below)."
+      RenderBlock (anonymous) at (0,52) size 784x23
+        RenderTextControl {INPUT} at (2,2) size 125x19 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
+        RenderText {#text} at (0,0) size 0x0
+layer at (13,65) size 119x13
+  RenderBlock {DIV} at (3,3) size 119x13
+caret: position 0 of child 0 {DIV} of child 3 {INPUT} of body
diff --git a/LayoutTests/platform/mac/fast/images/imagemap-focus-ring-expected.checksum b/LayoutTests/platform/mac/fast/images/imagemap-focus-ring-expected.checksum
new file mode 100644 (file)
index 0000000..767dbe0
--- /dev/null
@@ -0,0 +1 @@
+c4ec4eae0407f510da84a6a2242f1322
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/fast/images/imagemap-focus-ring-expected.png b/LayoutTests/platform/mac/fast/images/imagemap-focus-ring-expected.png
new file mode 100644 (file)
index 0000000..8930283
Binary files /dev/null and b/LayoutTests/platform/mac/fast/images/imagemap-focus-ring-expected.png differ
diff --git a/LayoutTests/platform/mac/fast/images/imagemap-focus-ring-expected.txt b/LayoutTests/platform/mac/fast/images/imagemap-focus-ring-expected.txt
new file mode 100644 (file)
index 0000000..067f367
--- /dev/null
@@ -0,0 +1,16 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 724x36
+          text run at (0,0) width 724: "Assuming the port-specific theme draws focus rings, this test can be used to ensure that a focus ring is drawn for an"
+          text run at (0,18) width 624: "imagemap. This test PASSED if a focus ring is drawn around the <area> in the imagemap (below)."
+      RenderBlock (anonymous) at (0,52) size 784x128
+        RenderInline {MAP} at (0,0) size 0x0
+          RenderText {#text} at (0,0) size 0x0
+          RenderText {#text} at (0,0) size 0x0
+        RenderText {#text} at (0,0) size 0x0
+        RenderImage {IMG} at (0,0) size 128x128
+        RenderText {#text} at (0,0) size 0x0
diff --git a/LayoutTests/platform/mac/fast/inline/inline-focus-ring-expected.checksum b/LayoutTests/platform/mac/fast/inline/inline-focus-ring-expected.checksum
new file mode 100644 (file)
index 0000000..2aeb8a9
--- /dev/null
@@ -0,0 +1 @@
+0a5e3258f957948cbeb0dae28175f69e
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/fast/inline/inline-focus-ring-expected.png b/LayoutTests/platform/mac/fast/inline/inline-focus-ring-expected.png
new file mode 100644 (file)
index 0000000..d399a85
Binary files /dev/null and b/LayoutTests/platform/mac/fast/inline/inline-focus-ring-expected.png differ
diff --git a/LayoutTests/platform/mac/fast/inline/inline-focus-ring-expected.txt b/LayoutTests/platform/mac/fast/inline/inline-focus-ring-expected.txt
new file mode 100644 (file)
index 0000000..5844df2
--- /dev/null
@@ -0,0 +1,15 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 784x36
+          text run at (0,0) width 784: "Assuming the port-specific theme draws focus rings, this test can be used to ensure that a focus ring is drawn for a hyperlink."
+          text run at (0,18) width 459: "This test PASSED if a focus ring is drawn around the hyperlink (below)."
+      RenderBlock (anonymous) at (0,52) size 784x18
+        RenderInline {A} at (0,0) size 64x18 [color=#0000EE]
+          RenderText {#text} at (0,0) size 64x18
+            text run at (0,0) width 64: "Hyperlink"
+        RenderText {#text} at (0,0) size 0x0
+caret: position 0 of child 0 {#text} of child 3 {A} of body
index b970098..04179e2 100644 (file)
@@ -1,3 +1,34 @@
+2010-10-14  Daniel Bates  <dbates@rim.com>
+
+        Reviewed by Darin Adler.
+
+        Only draw focus ring in RenderInline and RenderImage if the theme
+        is not able to draw a focus ring
+        https://bugs.webkit.org/show_bug.cgi?id=47632
+
+        Fixes an issue where RenderInline::paintOutline() and RenderImage::paintFocusRings()
+        would draw a focus ring regardless of whether the port-specific theme is able to
+        draw a focus ring. Instead, these methods should only draw a focus ring if the
+        theme is unable to draw a focus ring.
+
+        Also, extracted common focus ring drawing code from RenderObject::paintOutline()
+        and RenderInline::paintOutline() into RenderObject::paintFocusRing().
+
+        Tests: fast/forms/textfield-focus-ring.html
+               fast/images/imagemap-focus-ring.html
+               fast/inline/inline-focus-ring.html
+
+        * rendering/RenderImage.cpp:
+        (WebCore::RenderImage::paintFocusRings): Modified to only draw a focus ring if the
+        the theme does not draw one.
+        * rendering/RenderInline.cpp:
+        (WebCore::RenderInline::paintOutline): Modified to call RenderObject::paintFocusRing().
+        * rendering/RenderObject.cpp:
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::paintFocusRing): Added.
+        (WebCore::RenderObject::paintOutline): Modified to call RenderObject::paintFocusRing().
+        * rendering/RenderObject.h:
+
 2010-10-14  Pavel Feldman  <pfeldman@chromium.org>
 
         Reviewed by Yury Semikhatsky.
index f889926..84feb0f 100644 (file)
@@ -323,6 +323,9 @@ void RenderImage::paintFocusRings(PaintInfo& paintInfo, const RenderStyle* style
     
     RefPtr<HTMLCollection> areas = mapElement->areas();
     unsigned numAreas = areas->length();
+
+    if (theme()->supportsFocusRing(style))
+        return; // The theme draws the focus ring.
     
     // FIXME: Clip the paths to the image bounding box.
     for (unsigned k = 0; k < numAreas; ++k) {
index 2eb275c..6ee0e80 100644 (file)
@@ -993,15 +993,10 @@ void RenderInline::paintOutline(GraphicsContext* graphicsContext, int tx, int ty
     
     RenderStyle* styleToUse = style();
     if (styleToUse->outlineStyleIsAuto() || hasOutlineAnnotation()) {
-        int ow = styleToUse->outlineWidth();
-        Color oc = styleToUse->visitedDependentColor(CSSPropertyOutlineColor);
-
-        Vector<IntRect> focusRingRects;
-        addFocusRingRects(focusRingRects, tx, ty);
-        if (styleToUse->outlineStyleIsAuto())
-            graphicsContext->drawFocusRing(focusRingRects, ow, styleToUse->outlineOffset(), oc);
-        else
-            addPDFURLRect(graphicsContext, unionRect(focusRingRects));
+        if (!theme()->supportsFocusRing(styleToUse)) {
+            // Only paint the focus ring by hand if the theme isn't able to draw the focus ring.
+            paintFocusRing(graphicsContext, tx, ty, styleToUse);
+        }
     }
 
     if (styleToUse->outlineStyleIsAuto() || styleToUse->outlineStyle() == BNONE)
index 8be614d..d4ff8ce 100644 (file)
@@ -1151,6 +1151,16 @@ void RenderObject::drawArcForBoxSide(GraphicsContext* graphicsContext, int x, in
     }
 }
 #endif
+    
+void RenderObject::paintFocusRing(GraphicsContext* context, int tx, int ty, RenderStyle* style)
+{
+    Vector<IntRect> focusRingRects;
+    addFocusRingRects(focusRingRects, tx, ty);
+    if (style->outlineStyleIsAuto())
+        context->drawFocusRing(focusRingRects, style->outlineWidth(), style->outlineOffset(), style->visitedDependentColor(CSSPropertyOutlineColor));
+    else
+        addPDFURLRect(context, unionRect(focusRingRects));
+}        
 
 void RenderObject::addPDFURLRect(GraphicsContext* context, const IntRect& rect)
 {
@@ -1181,12 +1191,7 @@ void RenderObject::paintOutline(GraphicsContext* graphicsContext, int tx, int ty
     if (styleToUse->outlineStyleIsAuto() || hasOutlineAnnotation()) {
         if (!theme()->supportsFocusRing(styleToUse)) {
             // Only paint the focus ring by hand if the theme isn't able to draw the focus ring.
-            Vector<IntRect> focusRingRects;
-            addFocusRingRects(focusRingRects, tx, ty);
-            if (styleToUse->outlineStyleIsAuto())
-                graphicsContext->drawFocusRing(focusRingRects, ow, offset, oc);
-            else
-                addPDFURLRect(graphicsContext, unionRect(focusRingRects));
+            paintFocusRing(graphicsContext, tx, ty, styleToUse);
         }
     }
 
index 5b30177..ac92da4 100644 (file)
@@ -751,6 +751,7 @@ protected:
     // Overrides should call the superclass at the start
     virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
 
+    void paintFocusRing(GraphicsContext*, int tx, int ty, RenderStyle*);
     void paintOutline(GraphicsContext*, int tx, int ty, int w, int h);
     void addPDFURLRect(GraphicsContext*, const IntRect&);