Summary: when word-spacing is positive and the selection begins with a space, the highlight is not the right width (does not take into account the width of the initial space). To reproduce: open the testcase in Safari, click in the space next to the M and drag from there to the right. Expected: highlight to cover the entire space once the cursor crosses the middle of the space, and to extend over the letters one at a time as they are reached. Actual: once the cursor crosses the middle of the space, a thin highlight appears near the M. As the cursor is dragged over the letters, the highlight becomes wider.
Created attachment 3586 [details] testcase
Created attachment 3587 [details] suggested fix
Comment on attachment 3587 [details] suggested fix Give floatWidth the preceding character so that it can account for word spacing. (It is never given the entire string for performance reasons, see font.cpp r1.20).
Comment on attachment 3587 [details] suggested fix Oops. Breaks some layout tests.
Comment on attachment 3587 [details] suggested fix Is one preceding character always good enough?
I think so. It's only used by widthForNextCharacter() in WebTextRenderer.m:2241 .
Sorry, that's line 2171 in WebTextRenderer.m. Looks like a lot of the code relies on the current behavior of not passing the preceding char (in a way that I don't know how to fix for negative word-spacing), so I'll try a different approach for the fix.
Created attachment 3589 [details] add leading word space to the selection rect
Comment on attachment 3589 [details] add leading word space to the selection rect This uses the same logic as WebTextRenderer's widthForNextCharacter to decide if word spacing should be added.
Comment on attachment 3589 [details] add leading word space to the selection rect Looks OK, although I'd like to see a local variable to avoid the 4 calls to textObject(). In fact, maybe that function should be made inline.
Created attachment 3606 [details] revised patch Made textObject() inline AND added a local variable to avoid calling it repeatedly from selectionRect().
I hate word-spacing :-/ Exists in ToT, as at 27/8/05
Comment on attachment 3606 [details] revised patch Once textObject() is inlined, it needs to be in the header.
Created attachment 3623 [details] revised patch Moved textObject() to the header (couldn't define it inside the InlineTextBox definition since it needs RenderText to be defined).
Comment on attachment 3623 [details] revised patch r=me