[Webkit-unassigned] [Bug 117728] [EFL][WK2] Implement unit test callback: onSettingChange

[bugzilla-daemon at webkit.org]

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


Christophe Dumez <dchris at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #204892|review?                     |review-
               Flag|                            |




--- Comment #2 from Christophe Dumez <dchris at gmail.com>  2013-06-18 02:56:07 PST ---
(From update of attachment 204892)
View in context: https://bugs.webkit.org/attachment.cgi?id=204892&action=review

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:238
> +            break;

maybe we could return here and add a "FAIL();" after the loop to make sure we found the item?

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:283
> +            selectContextMenuOption(ewk_context_menu_item_submenu_get(item), EWK_CONTEXT_MENU_ITEM_TAG_CHECK_SPELLING_WHILE_TYPING, EWK_CHECKABLE_ACTION_TYPE);

why not pass the item directly to selectContextMenuOption() instead of iterating over the list twice?

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:284
> +            break;

same as before, would be nice to return early here and FAIL after the loop.

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:373
> +    isSettingEnabled = ewk_text_checker_continuous_spell_checking_enabled_get();

those 2 statements should be merged into one.

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:409
> +    ASSERT_FALSE(waitUntilTrue(callbacksExecutionStats.settingChange, defaultTimeoutInSeconds));

this looks weird. are we waiting to make sure the callback is *not* called? if so, isn't is terribly slow? also, the last arg is likely optional.

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:426
> +    ASSERT_FALSE(waitUntilTrue(callbacksExecutionStats.settingChange, defaultTimeoutInSeconds));

ditto.

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