Bug 16843
| Summary: | RenderText::addLineBoxRects erroneously includes last char for boundingBox | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Finnur Thorarinsson <finnur.webkit> |
| Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED DUPLICATE | ||
| Severity: | Normal | CC: | eric |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
Finnur Thorarinsson
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)
*** Bug 16844 has been marked as a duplicate of this bug. ***
Eric Seidel (no email)
*** This bug has been marked as a duplicate of 16844 ***