[Webkit-unassigned] [Bug 27889] [Chromium] RTL autocomplete popup is not layout correctly.

[bugzilla-daemon at webkit.org]

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





--- Comment #19 from Xiaomei Ji <xji at chromium.org>  2009-08-10 19:30:16 PDT ---
Hi Jay,

Thanks for your comments!
Please see my reply inline.

(In reply to comment #18)
> Here are my comments on the patch:
> 
> WebCore/platform/chromium/PopupMenuChromium.cpp
> I think it would be better to make the RTL detection behavior not dependent on
> m_settings.restrictWidthOfListBox and instead being a new parameter of
> m_settings.
> The idea would be to keep PopupMenuChromium generic and let the client decides
> which heuristic to use.

Make sense. I've updated.

> 
> In paintRow() regarding the ellipsis code. That code seems non-specific to the
> autofill. Would it make sense to have
> GraphicsContext::drawBidiTextWithinWidth() draw the ellipsis (may be the method
> could have an extra-parameter specifying whether or not an ellipsis is
> desired)?
> I am assuming the ellipsis ... is the same in all languages. If it is not, may
> be the extra-param could be a string of the ellipsis to use? (empty string for
> no ellipsis)

Make sense. I've moved the code inside
GraphaicsContext::drawBidiText(WithinWidth).

And per Mitz's suggestion, I used StringTruncator instead of manually
truncating the string and placing ellipses. The code is much elegant. One thing
to note is that in StringTruncator, horizontal ellipses (U+2026) will always be
drawn when text is truncated. I think that should be fine.

Thanks,
Xiaomei

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