Bug 87481 – [EFL] EFL's LayoutTestController does not support setTextDirection

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Chris Dumez]

EFL's LayoutTestController does not support setTextDirection which is tested by the following test case:
  fast/html/set-text-direction.html

[Chris Dumez]

Created attachment 144022 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 144022 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=144022&action=review

> Source/WebKit/efl/ewk/ewk_view.cpp:3992
> +    WebCore::Frame* focusedFrame = priv->page->focusController()->focusedFrame();

Is focusedFrameOrMainFrame() better than focusedFrame() so that there is no focusedFrame() ?

[Chris Dumez]

Created attachment 147787 [details]
Patch

Take feedback into consideration and rebase on master.

[Hajime Morrita]

Comment on attachment 147787 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147787&action=review

> Source/WebKit/efl/ewk/ewk_view.cpp:3976
> +    if (!editor || !editor->canEdit())

Frame always has an editor object. You don't need to check the nullity.

> Source/WebKit/efl/ewk/ewk_view.cpp:3979
> +    switch (direction) {

Maybe you adopt assert-matching-enums pattern which is used in chromium.
Then you don't need to duplicate these switch statements.
http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/src/AssertMatchingEnums.cpp

[Chris Dumez]

Created attachment 148260 [details]
Patch

Take morrita's feedback into consideration.

[WebKit Review Bot]

Comment on attachment 148260 [details]
Patch

Clearing flags on attachment: 148260

Committed r120692: <http://trac.webkit.org/changeset/120692>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.