[Qt] Gesture tap highlighter needs to take overflow clip into account.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 May 2012 07:37:01 +0000 (07:37 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 May 2012 07:37:01 +0000 (07:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=84989

Patch by Zalan Bujtas <zbujtas@gmail.com> on 2012-05-18
Reviewed by Kenneth Rohde Christiansen.

.:

* ManualTests/qt/tap-highlighting-overflow-hidden.html: Added.

Source/WebCore:

Apply overflow clip on the focus ring if needed.

Follow up patches are needed to address the following cases.

[Qt] Gesture tap highlighter should take parent iframe's transform into account.
https://bugs.webkit.org/show_bug.cgi?id=86645

[Qt] Gesture tap highlighter needs to take frame clipping into account.
https://bugs.webkit.org/show_bug.cgi?id=86646

[Qt] Gesture tap highlighter's overflow clip is not always correct when
nested enclosing containers have transforms.
https://bugs.webkit.org/show_bug.cgi?id=86641

Manual test: ManualTests/qt/tap-highlighting-overflow-hidden.html

* page/GestureTapHighlighter.cpp:
(WebCore::GestureTapHighlighter::pathForNodeHighlight):

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

ChangeLog
ManualTests/qt/tap-highlighting-overflow-hidden.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/GestureTapHighlighter.cpp

index ff732be..0c30da1 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2012-05-18  Zalan Bujtas  <zbujtas@gmail.com>
+
+        [Qt] Gesture tap highlighter needs to take overflow clip into account.
+        https://bugs.webkit.org/show_bug.cgi?id=84989
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        * ManualTests/qt/tap-highlighting-overflow-hidden.html: Added.
+
 2012-05-18  Christophe Dumez  <christophe.dumez@intel.com>
 
         [EFL] Add simple implementation of Web Intents
diff --git a/ManualTests/qt/tap-highlighting-overflow-hidden.html b/ManualTests/qt/tap-highlighting-overflow-hidden.html
new file mode 100644 (file)
index 0000000..2661d36
--- /dev/null
@@ -0,0 +1,87 @@
+<html>
+<head>
+<style type="text/css">
+
+    div {
+        position: absolute;
+        border: solid;
+        width: 100px;
+        height: 100px;
+    }
+
+    div.clipping {
+        color: red;
+        overflow: hidden;
+    }
+
+    div.noClipping {
+        color: green;
+        overflow: visible;
+        left: 10px;
+        top: 50px;
+    }
+
+    div.noTransform {
+        left: 150px;
+        top: 50px;
+    }
+
+    div.withTransform {
+        left: 300px;
+        top: 50px;
+        -webkit-transform: rotate(20deg);
+    }
+
+    div.outer {
+        left: 450px;
+        top: 50px;
+        -webkit-transform: rotate(-20deg);
+    }
+
+    div.inner {
+        left: 20px;
+        top: 10px;
+    }
+
+    iframe {
+        border: solid;
+        position: absolute;
+        width: 100px;
+        height: 100px;
+        left: 200px;
+        top: 300px;
+    }
+</style>
+</head>
+
+<body>
+Click on the links. Highlight should be clipped at the red border, while it should overflow on the green.<br>
+<div class="noClipping">
+    <a href="#">Div with overflow visible. Lorem ipsum dolor sit amet, consectetur adipisicing...</a>
+</div>
+<div class="clipping noTransform">
+    <a href="#">Div with overflow hidden without transform. Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</a>
+</div>
+<div class="clipping withTransform">
+    <a href="#">Div with overflow hidden with transform. Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</a>
+</div>
+<br>
+
+<div class="clipping outer">
+<div class="clipping inner">
+    <a href="#">Nested divs with overflow hidden with transform. Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</a>
+</div>
+</div>
+<br>
+<iframe src="data:text/html,
+    <style> div.overflowHiddenNoTransformi { color: red; border: solid; overflow: hidden; left: 150px; top: 100px; width: 100px; height: 100px; }</style>
+    <body>
+      Inside an iframe.
+  <div class='overflowHiddenNoTransformi'>
+    <a href='#'>Div with overflow hidden without transform. Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</a>
+  </div>
+    </body>
+    "></iframe>
+</body>
+</html>
+
index d94f1e5..58bfc76 100644 (file)
@@ -1,3 +1,29 @@
+2012-05-18  Zalan Bujtas  <zbujtas@gmail.com>
+
+        [Qt] Gesture tap highlighter needs to take overflow clip into account.
+        https://bugs.webkit.org/show_bug.cgi?id=84989
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        Apply overflow clip on the focus ring if needed.
+
+        Follow up patches are needed to address the following cases.
+
+        [Qt] Gesture tap highlighter should take parent iframe's transform into account.
+        https://bugs.webkit.org/show_bug.cgi?id=86645
+
+        [Qt] Gesture tap highlighter needs to take frame clipping into account.
+        https://bugs.webkit.org/show_bug.cgi?id=86646
+
+        [Qt] Gesture tap highlighter's overflow clip is not always correct when
+        nested enclosing containers have transforms.
+        https://bugs.webkit.org/show_bug.cgi?id=86641
+
+        Manual test: ManualTests/qt/tap-highlighting-overflow-hidden.html
+
+        * page/GestureTapHighlighter.cpp:
+        (WebCore::GestureTapHighlighter::pathForNodeHighlight):
+
 2012-05-17  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] KURL::fileSystemPath() should strip the query of the uri
index 0cb694f..7bd5db9 100644 (file)
@@ -39,6 +39,7 @@
 #include "Page.h"
 #include "RenderBoxModelObject.h"
 #include "RenderInline.h"
+#include "RenderLayer.h"
 #include "RenderObject.h"
 
 namespace WebCore {
@@ -134,16 +135,16 @@ inline void addHighlightRect(Path& path, const LayoutRect& rect, const LayoutRec
             contains(next, rect.maxX()) ? squared : rounded);
 }
 
-Path pathForRenderer(RenderObject* o)
+Path absolutePathForRenderer(RenderObject* const o)
 {
     ASSERT(o);
-    Path path;
 
     Vector<IntRect> rects;
-    o->addFocusRingRects(rects, /* acc. offset */ ownerFrameToMainFrameOffset(o));
+    LayoutPoint frameOffset = ownerFrameToMainFrameOffset(o);
+    o->addFocusRingRects(rects, frameOffset);
 
     if (rects.isEmpty())
-        return path;
+        return Path();
 
     // The basic idea is to allow up to three different boxes in order to highlight
     // text with line breaks more nicer than using a bounding box.
@@ -184,12 +185,51 @@ Path pathForRenderer(RenderObject* o)
             drawableRects.append(rects.last());
     }
 
+    // Clip the overflow rects if needed, before the ring path is formed to
+    // ensure rounded highlight rects. This clipping has the problem with nested
+    // divs with transforms, which could be resolved by proper Path::intersecting.
+    for (int i = drawableRects.size() - 1; i >= 0; --i) {
+        LayoutRect& ringRect = drawableRects.at(i);
+        LayoutPoint ringRectLocation = ringRect.location();
+
+        ringRect.moveBy(-frameOffset);
+
+        RenderLayer* layer = o->enclosingLayer();
+        RenderObject* currentRenderer = o;
+
+        // Check ancestor layers for overflow clip and intersect them.
+        while (layer) {
+            RenderObject* layerRenderer = layer->renderer();
+
+            if (layerRenderer->hasOverflowClip() && layerRenderer != currentRenderer) {
+                ringRect.move(currentRenderer->offsetFromAncestorContainer(layerRenderer));
+                currentRenderer = layerRenderer;
+
+                ASSERT(layerRenderer->isBox());
+                ringRect.intersect(toRenderBox(layerRenderer)->borderBoxRect());
+
+                if (ringRect.isEmpty())
+                    break;
+            }
+            layer = layer->parent();
+        }
+
+        if (ringRect.isEmpty()) {
+            drawableRects.remove(i);
+            continue;
+        }
+        // After clipping, reset the original position so that parents' transforms apply correctly.
+        ringRect.setLocation(ringRectLocation);
+    }
+
+    Path path;
     for (size_t i = 0; i < drawableRects.size(); ++i) {
         LayoutRect prev = i ? drawableRects.at(i - 1) : LayoutRect();
         LayoutRect next = i < (drawableRects.size() - 1) ? drawableRects.at(i + 1) : LayoutRect();
         addHighlightRect(path, drawableRects.at(i), prev, next);
     }
 
+    path.transform(localToAbsoluteTransform(o));
     return path;
 }
 
@@ -201,14 +241,10 @@ Path pathForNodeHighlight(const Node* node)
 {
     RenderObject* renderer = node->renderer();
 
-    Path path;
     if (!renderer || (!renderer->isBox() && !renderer->isRenderInline()))
-        return path;
-
-    path = pathForRenderer(renderer);
-    path.transform(localToAbsoluteTransform(renderer));
+        return Path();
 
-    return path;
+    return absolutePathForRenderer(renderer);
 }
 
 } // namespace GestureTapHighlighter