[Webkit-unassigned] [Bug 24168] RTL: Home/End key does not behave correctly in mixed bidi text in RTL document

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 22 17:15:14 PDT 2009


https://bugs.webkit.org/show_bug.cgi?id=24168





------- Comment #11 from mitz at webkit.org  2009-04-22 17:15 PDT -------
(In reply to comment #10)

> > +        if (!startRenderer) {
> > +            startNode = NULL;
> > +            return;
> > +        }
> > 
> > Can this ever happen? I know that there is other old code checking for this,
> > but I don't think this case can happen. Maybe the code should assert that it
> > doesn't happen. Again, if you leave this code in, please use 0 instead of NULL.
> 
> I will run layout test against it to check whether it happens.
> But I see checking whether renderer exists in many places of the code. I am
> thinking probably to leave it there. 
> Or renderer of leave box is different, and should *never* be NULL?

A Node's renderer() can be null, but, as far as I can tell, an InlineBox's
renderer() can never be null. Most places in WebCore that access an InlineBox's
render() don't null check.

> > +    int startOffset = 0;
> > +    if (logicStartBox->isInlineTextBox()) {
> > +        InlineTextBox *startTextBox = static_cast<InlineTextBox
> > *>(logicStartBox);
> > +        startOffset = startTextBox->start();
> > +    }
> > 
> > I think you can use InlineBox::caretMinOffset() here instead of special-casing
> > InlineTextBox.
> > 
> 
> Do you mean change the above to "int startOffset =
> logicalStartBox->caretMinOffset()"?

Yes.

> Then, is it the same as the above logic?
> Could leaf box be InlineRunBox or InlineFlowBox? In which cases, 
> will the caretMinOffset() be 0?

An empty InlineFlowBox can be a leaf, in which case you will get 0.

> I am a bit confused about those virtual functions in InlineBox (and its derived
> classes) and RenderObject (and its derived classes). Who will use 
> RenderText::caretMaxOffset()?

caretMaxOffset() in htmlediting.cpp looks like one place that calls it.

> > +    int endOffset = 1;
> > +    if (logicEndNode->hasTagName(brTag)) {
> > +        endOffset = 0;
> > +    } else if (logicEndBox->isInlineTextBox()) {
> > +        InlineTextBox *endTextBox = static_cast<InlineTextBox *>(logicEndBox);
> > +        endOffset = endTextBox->start();
> > +        if (!endTextBox->isLineBreak())
> > +            endOffset += endTextBox->len();
> > +    }
> > 
> > And here you can use InlineBox::caretMaxOffset() in the else clause, instead of
> > special-casing InlineTextBox. Can you use caretMaxOffset() to get the desired
> > result for a <br> too?
> > 
> 
> Even for the "else if" case (the InlineTextBox), is the above logic the same as
> caretMaxOffset()?
> caretMaxOffset() returns m_start + m_len for InlineTextBox. If the InlineTexBox
> isLineBreak(), is its m_len == 0? I did not find the related code by just quick
> browsing.

Oh, I think you are right! For pre-wrapped text, the \n will have an
InlineTextBox with m_len == 1, but you want the offset before the newline
character.

> I have to admit that I copied most of the logic from startOfLine/endOfLine by
> understanding the main part of them, but not every path. 
> I did not quite understand the former logic, and I forgot whether it is ever
> been hit by the test. As to the latter logic, I think it is hit by the code,
> and that is why I changed isSameLine() to isSameLogicLine().
> I will run test again to find out more.

I think some of that logic my actually apply to home/end but *not* to the
actual current behavior of startOfLine() and endOfLine(), and it was just
written assuming that the logical and visual orders are the same. You may be
able to find cases where startOfLine() and endOfLine() fail with RTL or
bidirectional text because of the wrong assumptions.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list