Bug 56854 – computation in InlineTextBox::positionForOffset() might be wrong.

• Component: Layout and Rendering
• Status: RESOLVED CONFIGURATION CHANGED

[Xiaomei Ji]

Following line inside "float InlineTextBox::positionForOffset(int offset) const" might be wrong:


    // FIXME: Do we need to add rightBearing here?
    return f.selectionRectForText(TextRun(text->text()->characters() + m_start, m_len, textRenderer()->allowTabs(), textPos(), m_expansion, trailingExpansionBehavior(), !isLeftToRightDirection(), m_dirOverride),
                                  IntPoint(logicalLeft(), 0), 0, from, to).maxX();


convert "logicalLeft()" to "int" in "IntPoint(logicalLeft(), 0)" should not be done.

For example,
    <textarea id="textarea_rtl_no_wrap" style="padding:0; margin:0; white-space:nowrap; width: 100px; font: 8px" dir="rtl">&#1491;&#1490;&#1500;&#1495;&#1499; &#1490;&#1491;&#1499; &#1500;&#1495;&#1497;&#1491;&#1490;&#1499; &#1497;&#1495;&#1506;&#1491;&#1491;</textarea>

the width of the text is 120, the logicalLeft() is -19.775, which is round to -19. Then, the positionForOffset(0) will be "120 - 19 = 101" which is greater than the textarea width: 100px.

[Xiaomei Ji]

Created attachment 88331 [details]
patch w/ layout test

Dan, Enrica,

Could you please help on verifying the layout test result on editing/selection/mixed-editability-10.html and platform/mac/editing/input/caret-primary-bidi.html?

I tried moving caret in caret-primary-bidi.html, and I did not see any where that the caret becomes invisible. Seems the result is acceptable.
But I am not sure on mixed-editability-10.html. The result changed for the first <div> with one regression and one progression.

[Xiaomei Ji]

The patch is not completed.
It breaks mixed-editability-10.html in the sense that when clicking left of the span, the caret overlaps with the span, the caret should be placed before the span.

The code in the patch itself seems reasonable.
But there are probably some other code pieces that do float/int conversion or float rounding or other float hacking that need to be taken care of.

[Ryosuke Niwa]

Comment on attachment 88331 [details]
patch w/ layout test

View in context: https://bugs.webkit.org/attachment.cgi?id=88331&action=review

> LayoutTests/platform/mac/editing/input/caret-primary-bidi-expected.txt:3
> -0: 124,508,0,28
> +0: 125,508,0,28
>  1: 21,564,0,28
>  2: 36,564,0,28

These output makes no sense to me.  We should come up with a better output here.

[Ahmad Saleem]

@Alan - I looked into it and it seems that it is in IFC part and I am sure that you might have already fixed it and improved it.


Anything to do it?

https://searchfox.org/wubkat/source/Source/WebCore/layout/integration/inline/InlineIteratorTextBox.cpp#93

[Ahmad Saleem]

(In reply to Ahmad Saleem from comment #4)
> @Alan - I looked into it and it seems that it is in IFC part and I am sure
> that you might have already fixed it and improved it.
> 
> 
> Anything to do it?
> 
> https://searchfox.org/wubkat/source/Source/WebCore/layout/integration/inline/
> InlineIteratorTextBox.cpp#93

Code is now deleted - https://commits.webkit.org/244347@main

and can't find reference of this comment in IFC.