Title: [133294] trunk
Revision
133294
Author
michael.brun...@digia.com
Date
2012-11-02 07:25:35 -0700 (Fri, 02 Nov 2012)

Log Message

[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.

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:

Modified Paths

Added Paths

Diff

Modified: trunk/ChangeLog (133293 => 133294)


--- trunk/ChangeLog	2012-11-02 14:07:45 UTC (rev 133293)
+++ trunk/ChangeLog	2012-11-02 14:25:35 UTC (rev 133294)
@@ -1,3 +1,16 @@
+2012-11-02  Michael BrĂ¼ning  <michael.brun...@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  <fisch...@chromium.org>
 
         HTMLMediaPlayer should free m_player when src is set/changed

Added: trunk/ManualTests/tap-gesture-on-em-link-tap-highlight-assert.html (0 => 133294)


--- trunk/ManualTests/tap-gesture-on-em-link-tap-highlight-assert.html	                        (rev 0)
+++ trunk/ManualTests/tap-gesture-on-em-link-tap-highlight-assert.html	2012-11-02 14:25:35 UTC (rev 133294)
@@ -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=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (133293 => 133294)


--- trunk/Source/WebCore/ChangeLog	2012-11-02 14:07:45 UTC (rev 133293)
+++ trunk/Source/WebCore/ChangeLog	2012-11-02 14:25:35 UTC (rev 133294)
@@ -1,3 +1,22 @@
+2012-11-02  Michael BrĂ¼ning  <michael.brun...@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  <arpitabahug...@gmail.com>
 
         Regression r130057: Improper preferred width calculation when an inline replaced object, wrapped in an inline flow, follows some text.

Modified: trunk/Source/WebCore/page/GestureTapHighlighter.cpp (133293 => 133294)


--- trunk/Source/WebCore/page/GestureTapHighlighter.cpp	2012-11-02 14:07:45 UTC (rev 133293)
+++ trunk/Source/WebCore/page/GestureTapHighlighter.cpp	2012-11-02 14:25:35 UTC (rev 133294)
@@ -165,9 +165,13 @@
     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 @@
     // 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();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to