RESOLVED DUPLICATE of bug 16844 16843
RenderText::addLineBoxRects erroneously includes last char for boundingBox 
https://bugs.webkit.org/show_bug.cgi?id=16843
Summary RenderText::addLineBoxRects erroneously includes last char for boundingBox
Finnur Thorarinsson
Reported 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);
Attachments
Add attachment proposed patch, testcase, etc.
Mark Rowe (bdash)
Comment 1 2008-01-11 11:30:52 PST
*** Bug 16844 has been marked as a duplicate of this bug. ***
Eric Seidel (no email)
Comment 2 2008-01-11 11:34:36 PST
*** This bug has been marked as a duplicate of 16844 ***
Note You need to log in before you can comment on or make changes to this bug.