[Webkit-unassigned] [Bug 61931] WebKitTestRunner needs an implementation of setTextDirection
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jun 24 03:09:11 PDT 2011
https://bugs.webkit.org/show_bug.cgi?id=61931
Hironori Bono <hbono at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #98321|0 |1
is obsolete| |
Attachment #98476| |review?
Flag| |
--- Comment #8 from Hironori Bono <hbono at chromium.org> 2011-06-24 03:09:11 PST ---
Created an attachment (id=98476)
--> (https://bugs.webkit.org/attachment.cgi?id=98476&action=review)
Patch v4 (applied comments)
Greeting Adam,
Thank you for your review and comments.
I have updated my change to apply your comments. (I may misunderstand your comments, though.)
(In reply to comment #7)
> (From update of attachment 98321 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=98321&action=review
>
> Thanks for implementing this! I think this could use another pass.
>
> > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp:327
> > +void InjectedBundle::setTextDirection(WebFrame* frame, const String& direction)
> > +{
> > + Frame* coreFrame = frame ? frame->coreFrame() : 0;
> > + if (!coreFrame || !coreFrame->editor())
> > + return;
> > +
> > + if (direction == "auto")
> > + coreFrame->editor()->setBaseWritingDirection(NaturalWritingDirection);
> > + else if (direction == "ltr")
> > + coreFrame->editor()->setBaseWritingDirection(LeftToRightWritingDirection);
> > + else if (direction == "rtl")
> > + coreFrame->editor()->setBaseWritingDirection(RightToLeftWritingDirection);
> > +}
>
> This function should be a member of WebFrame, not InjectedBundle. The corresponding API would be WKBundleFrameSetTextDirection.
I have added a new function WebFrame::setTextDirection and moved this code there. Even though I have renamed WKBundleSetTextDirection to WKBundleFrameSetTextDirection, I left this InjectedBundle::setTextDirection (it just calls WebFrame::setTextDirection) since I'm not sure if WKBundleFrameSetTextDirection should call WebFrame::setTextDirection directly.
> > Source/WebKit/win/Interfaces/IWebFramePrivate.idl:122
> > + HRESULT setTextDirection([in] BSTR direction);
>
> New functions must be added to the end of the interface to avoid breaking nightly builds.
Done. Thank you for noticing it.
> > Tools/DumpRenderTree/win/LayoutTestControllerWin.cpp:1578
> > + wstring directionWstring = jsStringRefToWString(direction);
> > + BSTR directionBSTR = SysAllocStringLen((OLECHAR*)directionWstring.c_str(), directionWstring.length());
> > + framePrivate->setTextDirection(directionBSTR);
> > + SysFreeString(directionBSTR);
>
> Please use the bstrT() helper function instead of doing this manually.
Done. Thank you for noticing it.
Regards,
Hironori Bono
--
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