FloatRect rect = absoluteQuadForTextBox(box, start, end, useSelectionHeight);
This is converting local to absolute twice.
Regression from http://trac.webkit.org/changeset/91763
Actually I think it's just an unfortunate (and incorrect) name. absoluteQuadForTextBox doesn't convert to absolute, I'll make sure this is the case and rename it to localQuadForTextBox.
Created attachment 103101 [details]
Comment on attachment 103101 [details]
Not sure this is right. quadForTextBox() calls selectionRect(), which uses selectionRectForRepaint(). The 'for repaint' indicates that it's already been converted to the coordinate system of the repaintContainer (or the root).
If there aren't repaint tests for this code, you should make some.
The change 91763 didn't change change any logic apart from replacing a check for (!r.isEmpty()) with (r.height()) to match absoluteRectsForRange. Just as before it calls selectionRect and then adjust the rect and converts it to the absolute coordinate space using localToAbsoluteQuad.
As far as I can tell quadForTextBox calls InlineTextBox::selectionRect which in turn returns the selection rect in the local coordinate space.
There are a couple of tests that cover this code already and they helped me catch a a mistake in the first version of my patch.
If you still do not trust this to be correct I'd gladly revert change 91763.
Ah, InlineTextBox::selectionRect() is in local coords, RenderObject::selectionRect() is in "repaint" coords. The former should be renamed ("localSelectionRect()"?)
(In reply to comment #6)
> Ah, InlineTextBox::selectionRect() is in local coords, RenderObject::selectionRect() is in "repaint" coords. The former should be renamed ("localSelectionRect()"?)
Makes sense, I'll make the change.
Created attachment 103128 [details]
Comment on attachment 103128 [details]
Clearing flags on attachment: 103128
Committed r92625: <http://trac.webkit.org/changeset/92625>
All reviewed patches have been landed. Closing bug.
RenderText::absoluteRectsForRange still has:
// FIXME: This code is wrong. It's converting local to absolute twice. http://webkit.org/b/65722
The confusing comment remains and I have no idea if it's true or not anymore (but it seems not).
It's still there :(
No longer: https://trac.webkit.org/changeset/219753/webkit