[Webkit-unassigned] [Bug 25321] RLE/LRE characters are removed from any text entered into a text box (either using a keyboard or by pasting)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 30 14:53:26 PDT 2010


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





--- Comment #12 from Xiaomei Ji <xji at chromium.org>  2010-08-30 14:53:25 PST ---
Thanks for the review!
It makes me realized that this is not the right fix because upstream/downstream are not symmetric when handling unicode bidi control characters.

I need to think it more and would appreciate if Dan could comment on the appropriate way of fixing this.

(In reply to comment #11)
> (From update of attachment 65626 [details])
> > +static bool unicodeBiDiControlCharacter(UChar ch)
> 
> This function name needs the word "is" in it, because it does not return a "Unicode bi di control character". It also makes no sense to capitalize the "D" in "bidi", which is short for "bidirectional".
> 
> > +                if (textOffset == box->start() + box->len() + 1 && unicodeBiDiControlCharacter((static_cast<Text*>(currentNode))->data()[textOffset - 1]))
> > +                    return currentPos;
> 
> This code needs a why comment. Why doe this one particular function need a special case for Unicode bidirectional control characters. Aren't the other characters that need this sort of special handling? What makes these bidirectional control characters so important.
> 
> >      // Insert the character at the leftmost candidate.
> > +    startPosition = startPosition.downstream();
> >      startPosition = startPosition.upstream();
> 
> This code needs a why comment. Otherwise someone would just remove the call to downstream.
> 
> These issues are enough for a review-, but I’d also like Dan Bernstein to comment on the appropriate way of fixing this.

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


More information about the webkit-unassigned mailing list