Bug 16843 - RenderText::addLineBoxRects erroneously includes last char for boundingBox
Summary: RenderText::addLineBoxRects erroneously includes last char for boundingBox
Status: RESOLVED DUPLICATE of bug 16844
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-01-11 11:04 PST by Finnur Thorarinsson
Modified: 2008-01-11 11:34 PST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Finnur Thorarinsson 2008-01-11 11:04:24 PST
Version: local build, downloaded from tarball on Jan 10th, 2008.
Platform and OS: Reproduced with Safari (which I hooked up to use my local build of WebKit) on Windows XP (didn't try Mac)

To reproduce, start with this super simple web page:

<html> 
<body> 
Ctrl-T<br>
asdf<br>
</body> 
</html>

Then add the following code line to Frame::findString at the bottom:

+    IntRect bounds = resultRange->boundingBox();
     return true;

... and set a breakpoint on the return statement.

Now, in Safari, when you incrementally search for 'asdf', you will see the following boundingBox as the breakpoint hits:

search (x, y)   (w,  h)
--------------------------
a      (8, 26)  (14, 42)
as     (8, 26)  (20, 42)
asd    (8, 26)  (33, 42)  <- clearly wrong, this is showing the boundingBox for 'asdf' instead of 'asd'
asdf   (8, 26)  (33, 42)

The reason for this error is that box->end() is assumed to point beyond the last character, when in fact it points *to* the last character (as specified in a comment in this function). WebKit uses box->end() in two locations in this function but only adds +1 to it in one location. The fix is therefore to add the missing +1. 

With the fix we get:

search (x, y)   (w,  h)
--------------------------
a      (8, 26)  (14, 42)
as     (8, 26)  (20, 42)
asd    (8, 26)  (28, 42)  <- correct
asdf   (8, 26)  (33, 42)

I've been in touch with Eric Seidel and will work with him on what else I need to supply for this bug (if anything).

In the meantime, here is the fix:

===================================================================
--- RenderText.cpp      (revision 29378)
+++ RenderText.cpp      (working copy)
@@ -211,7 +211,7 @@
     absolutePositionForContent(x, y);

     for (InlineTextBox* box = firstTextBox(); box; box = box->nextTextBox()) {
-        if (start <= box->start() && box->end() <= end) {
+        if (start <= box->start() && box->end() + 1 <= end) {
             IntRect r = IntRect(x + box->xPos(), y + box->yPos(), box->width(),
 box->height());
             if (useSelectionHeight) {
                 IntRect selectionRect = box->selectionRect(x, y, start, end);
Comment 1 Mark Rowe (bdash) 2008-01-11 11:30:52 PST
*** Bug 16844 has been marked as a duplicate of this bug. ***
Comment 2 Eric Seidel (no email) 2008-01-11 11:34:36 PST

*** This bug has been marked as a duplicate of 16844 ***