[webkit-reviews] review requested: [Bug 61272] [Chromium] On-the-spot IME support for windowless plug-ins : [Attachment 95899] Patch v2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jun 3 05:40:12 PDT 2011


Hironori Bono <hbono at chromium.org> has asked  for review:
Bug 61272: [Chromium] On-the-spot IME support for windowless plug-ins
https://bugs.webkit.org/show_bug.cgi?id=61272

Attachment 95899: Patch v2
https://bugs.webkit.org/attachment.cgi?id=95899&action=review

------- Additional Comments from Hironori Bono <hbono at chromium.org>
Greetings Kent-san,

Thank you for your comments.

(In reply to comment #6)
> (From update of attachment 95420 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=95420&action=review
  ...
> We had better use wtf/text/StringBuilder.

If I recall correctly, it is not so simple to append text to a StringBuffer
object as this code does. (I could be wrong, though.) Since this code is used
only when we type text with an IME, I'm a little wondering if we use
StringBuffer here and make this code more complicated. Would it be possible to
give me a good idea that can use StringBuffer without losing simplicity?

> > LayoutTests/platform/chromium/test_expectations.txt:4036
> > +
> > +// We need a Chromium-side change to fix this test.
> > +BUGCR82507 :
platform/chromium-win/plugins/windowless-plugin-ime-event-test.html = TEXT
> 
> I recommend adding lines not at the bottom of test_expectations.txt in order
to avoid patch conflicts.

Thank you for noticing it. I have moved it to the 'Plugin tests' section.

Regards,

Hironori Bono


More information about the webkit-reviews mailing list