[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 53e2b6536ff952a81225fe1eb57203a070f5ef5a..784e8ba065c6cbaaa8f23770818304a66289ef3f 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 b0e28ceab69f3fe9d9a3f1a99f379526b96f9924..71a2e6e3bb5f60f0ba5b3dc2fb464c665181260d 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 56df47c269945bdaa4530693cd7f328ae086bf04..6688d150151c89e75574eab8f8595cd7938575aa 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();