[webkit-reviews] review denied: [Bug 107840] [WTR][WK2] Implement testRunner.setSmartInsertDeleteEnabled : [Attachment 184973] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Manuel Rego Casasnovas
<rego at igalia.com>'s request for review:
Bug 107840: [WTR][WK2] Implement testRunner.setSmartInsertDeleteEnabled
https://bugs.webkit.org/show_bug.cgi?id=107840

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

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=184973&action=review


r- for not using Settings.in.

This seems to give us 2 ways to control SmartInsertDelete since Chromium still
has WebViewClient::isSmartInsertDeleteEnabled.	Maybe that's OK as long as
there's a transition plan.

> Source/WebCore/page/Settings.h:307
> +	   void setSmartInsertDeleteEnabled(bool);
> +	   bool smartInsertDeleteEnabled() { return m_smartInsertDeleteEnabled;
}

Please use Settings.in, which will generate all the Settings code for you (your
code doesn't bit pack properly).

> Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:929
> +void TestRunner::setSmartInsertDeleteEnabled(bool enabled)
> +{
> +   
WKBundlePageSetSmartInsertDeleteEnabled(InjectedBundle::shared().page()->page()
, enabled);
> +}

If you use Settings.in, it will automatically add this setter to
internals.settings.setSmartInsertDeleteEnabled.