[webkit-reviews] review requested: [Bug 61931] WebKitTestRunner needs an implementation of setTextDirection : [Attachment 98697] Patch v5

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 27 04:08:15 PDT 2011


Hironori Bono <hbono at chromium.org> has asked  for review:
Bug 61931: WebKitTestRunner needs an implementation of setTextDirection
https://bugs.webkit.org/show_bug.cgi?id=61931

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

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

Thank you for your review and comments. (This is a very good practice about
WebKitTestRunner for me.)

(In reply to comment #10)
> This function should be in WKBundleFrame.cpp, and shouldn't take a
WKBundleRef parameter.

Thank you for noticing it. I did not noticed WKBundleFrame.cpp. I have moved
this WKBundleFrameSetTextDirection. (It makes this change simpler.)

> This should be in WKBundleFramePrivate.h.

Done.

> You can remove the STDMETHODCALLTYPE (it's only needed on the declaration,
not the definition) and the /* [in] */ comment (even though most other
functions in this file have left them in place). We'd like to be as concise as
possible.

Thank you so much for your description. (I have pasted this code just because
every other function uses this style even though I'm not sure why.) This
description answers my question.

Regards,

Hironori Bono


More information about the webkit-reviews mailing list