[Webkit-unassigned] [Bug 109404] Add selectTrailingWhitespaceEnabled setting to WebCore::Page

[bugzilla-daemon at webkit.org]

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





--- Comment #27 from Manuel Rego Casasnovas <rego at igalia.com>  2013-03-06 01:46:50 PST ---
(In reply to comment #25)
> (From update of attachment 191512 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=191512&action=review
> 
> > Source/WebKit/chromium/public/WebViewClient.h:-190
> > -    virtual bool isSmartInsertDeleteEnabled() { return true; }
> > -    virtual bool isSelectTrailingWhitespaceEnabled() { return true; }
> 
> Is it ok to remove these public APIs from the Chromium port?

I agree that remove the API here and not add it in WebSettings.h doesn't seem right. I'll try to explain the issue deeply in the next paragraphs.

Now smartInsertDeleteEnabled and selectTrailingWhitespacesEnabled are going to be settings of WebCore::Page, this implies that in chromium there'll be 2 ways to control them since there will be yet WebViewClient::isSmartInsertDeleteEnabled and WebViewClient::isSelectTrailingWhitespaceEnabled.

So I guess that we have 2 options for chromium port:
a) Use smartInsertDeleteEnabled and selectTrailingWhitespacesEnabled from page settings:
  * This will imply changing the public API:
    * Removing WebViewClient::isSmartInsertDeleteEnabled and WebViewClient::isSelectTrailingWhitespaceEnabled.
    * Adding WebSettings::setSmartInsertDeleteEnabled and WebSettings::setSelectTrailingWhitespaceEnabled.
  * Anyway, I'm not sure if this should be here or in a separate patch. Maybe, chromium port prefers to plan the migration and keep working with WebViewClient API for a while. But I guess that eventually this change should be done.
b) Keep using WebViewClient API and try to fix DRT to use the page settings:
  * So, no changes in Source/WebKit/chromium/ would be needed.
  * DRT should still keep some code related to smartInsertDeleteEnabled and selectTrailingWhitespacesEnabled trying to use directly the page settings. If this is not possible, the related tests would fail in chromium port, as now they'll use internals.settings.setSmartInsertDeleteEnabled (which change directly the page setting) instead of testRunner.setSmartInsertDeleteEnabled and the page setting won't be used at all under Source/WebKit/chromium/.
  * Plan the migration for the future. I could provide a patch doing the changes described in option a) in a different bug and I guess it would be eventually integrated.

I'll upload a patch with the first solution (option a)), however I'm open to change whatever is needed following what chromium port people decide regarding to this issue. Moreover, I can be completely wrong in my assumptions so please let me know.

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