[Qt][WK2] ASSERT hit for every mouse click
authormichael.bruning@digia.com <michael.bruning@digia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Nov 2012 14:25:35 +0000 (14:25 +0000)
committermichael.bruning@digia.com <michael.bruning@digia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Nov 2012 14:25:35 +0000 (14:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=100607

Reviewed by Jocelyn Turcotte.

.:

Added a test with a link that contains an <em> tag surrounding the entire inner text.
The test should be run on an assert enabled build and the assert should not be
triggered when tapping the link.

* ManualTests/tap-gesture-on-em-link-tap-highlight-assert.html: Added.

Source/WebCore:

Changed the logic of absolutePathForRenderer to use the first highlight box as the mid box
by uniting the two in case the mid box is empty. This allows the first box to be merged with
the last box should they intersect, and thereby prevents an ASSERT in addHighlightRect that is
triggered by two intersecting boxes being passed to addHighlightRect as separate ones.

Also, this patch removes some superfluous checks for LayoutRect::isEmpty, which is being checked
in LayoutRect::intersects already.

No new tests, but added manual test: ManualTests/tap-gesture-on-em-link-tap-highlight-assert.html

* page/GestureTapHighlighter.cpp:

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

ChangeLog
ManualTests/tap-gesture-on-em-link-tap-highlight-assert.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/GestureTapHighlighter.cpp

index 53e2b65..784e8ba 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2012-11-02  Michael BrĂ¼ning  <michael.bruning@digia.com>
+
+        [Qt][WK2] ASSERT hit for every mouse click
+        https://bugs.webkit.org/show_bug.cgi?id=100607
+
+        Reviewed by Jocelyn Turcotte.
+
+        Added a test with a link that contains an <em> tag surrounding the entire inner text.
+        The test should be run on an assert enabled build and the assert should not be
+        triggered when tapping the link.
+
+        * ManualTests/tap-gesture-on-em-link-tap-highlight-assert.html: Added.
+
 2012-11-01  Ami Fischman  <fischman@chromium.org>
 
         HTMLMediaPlayer should free m_player when src is set/changed
diff --git a/ManualTests/tap-gesture-on-em-link-tap-highlight-assert.html b/ManualTests/tap-gesture-on-em-link-tap-highlight-assert.html
new file mode 100644 (file)
index 0000000..ed59249
--- /dev/null
@@ -0,0 +1,7 @@
+<html>
+<body>
+    <p>This test verifies that a link that starts with or contains an em tag does not assert in debug builds.</p>
+    <p style='color:green'>Tapping on the link should not trigger an assert.</p>
+    <div><a href="nothing.html"><em>Nothing.</em></a></div>
+</body>
+</html>
index b0e28ce..71a2e6e 100644 (file)
@@ -1,3 +1,22 @@
+2012-11-02  Michael BrĂ¼ning  <michael.bruning@digia.com>
+
+        [Qt][WK2] ASSERT hit for every mouse click
+        https://bugs.webkit.org/show_bug.cgi?id=100607
+
+        Reviewed by Jocelyn Turcotte.
+
+        Changed the logic of absolutePathForRenderer to use the first highlight box as the mid box 
+        by uniting the two in case the mid box is empty. This allows the first box to be merged with
+        the last box should they intersect, and thereby prevents an ASSERT in addHighlightRect that is
+        triggered by two intersecting boxes being passed to addHighlightRect as separate ones.
+
+        Also, this patch removes some superfluous checks for LayoutRect::isEmpty, which is being checked
+        in LayoutRect::intersects already.
+
+        No new tests, but added manual test: ManualTests/tap-gesture-on-em-link-tap-highlight-assert.html
+
+        * page/GestureTapHighlighter.cpp:
+
 2012-11-02  Arpita Bahuguna  <arpitabahuguna@gmail.com>
 
         Regression r130057: Improper preferred width calculation when an inline replaced object, wrapped in an inline flow, follows some text.
index 56df47c..6688d15 100644 (file)
@@ -165,9 +165,13 @@ Path absolutePathForRenderer(RenderObject* const o)
     LayoutRect first;
     LayoutRect last;
 
-    // Add the first box, but merge it with the center boxes if it intersects.
+    // Add the first box, but merge it with the center boxes if it intersects or if the center box is empty.
     if (rects.size() && !rects.first().isEmpty()) {
-        if (!mid.isEmpty() && mid.intersects(rects.first()))
+        // If the mid box is empty at this point, unite it with the first box. This allows the first box to be
+        // united with the last box if they intersect in the following check for last. Not uniting them would
+        // trigger in assert in addHighlighRect due to the first and the last box intersecting, but being passed
+        // as two separate boxes.
+        if (mid.isEmpty() || mid.intersects(rects.first()))
             mid.unite(rects.first());
         else {
             first = rects.first();
@@ -178,7 +182,7 @@ Path absolutePathForRenderer(RenderObject* const o)
     // Add the last box, but merge it with the center boxes if it intersects.
     if (rects.size() > 1 && !rects.last().isEmpty()) {
         // Adjust center boxes to boundary of last
-        if (!mid.isEmpty() && mid.intersects(rects.last()))
+        if (mid.intersects(rects.last()))
             mid.unite(rects.last());
         else {
             last = rects.last();