[Webkit-unassigned] [Bug 109404] Add selectTrailingWhitespaceEnabled setting to WebCore::Page
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Mar 7 01:09:03 PST 2013
https://bugs.webkit.org/show_bug.cgi?id=109404
--- Comment #30 from Manuel Rego Casasnovas <rego at igalia.com> 2013-03-07 01:11:24 PST ---
(In reply to comment #29)
> (From update of attachment 191692 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=191692&action=review
>
> > Source/WebCore/page/Settings.in:199
> > +# smartInsertDeleteEnabled and selectTrailingWhitespaceEnabled are mutually
> > +# exclusive, meaning that enabling one will disable the other.
>
> This seems like a new behavior and I'm not sure how you would enforce this for all ports. For example, in Chromium, the current behavior is:
>
> bool RenderViewImpl::isSmartInsertDeleteEnabled() {
> #if defined(OS_MACOSX)
> return true;
> #else
> return false;
> #endif
> }
>
> bool RenderViewImpl::isSelectTrailingWhitespaceEnabled() {
> #if defined(OS_WIN)
> return true;
> #else
> return false;
> #endif
> }
>
> Which means on Chromium Linux, both are false.
Maybe in Chromium is different, but I included the comment because of it's what is explained in WebCore::Editor:
// smartInsertDeleteEnabled and selectTrailingWhitespaceEnabled are
// mutually exclusive, meaning that enabling one will disable the other.
bool smartInsertDeleteEnabled();
bool isSelectTrailingWhitespaceEnabled();
Besides, the ports allowing to change these settings (like mac or win) where changing the other setting to the opposite value when a setting was established. Anyway I can remove the comment here if this is not true for all the ports.
> > Source/WebKit/chromium/public/WebSettings.h:150
> > + virtual void setSelectTrailingWhitespaceEnabled(bool) = 0;
>
> I don't think we need to add an API for this since I don't think we plan on changing these during runtime. The exception is for DRT, but that can use the WebCore Internals API.
Yes for DRT I'm already using the internals.
> > Source/WebKit/chromium/public/WebViewClient.h:-190
> > - virtual bool isSmartInsertDeleteEnabled() { return true; }
> > - virtual bool isSelectTrailingWhitespaceEnabled() { return true; }
>
> The problem with removing this is that we'll no longer get the correct behavior in Chromium, which overrides these virtual methods-- see the RenderViewImpl snippet above. As long as we retain the correct behavior, it's OK to remove this.
>
> There are a couple ways you could do this, (1) Find a good time to set this (maybe WebViewImpl's constructor?) or (2) set the right default value in Settings.cpp (e.g., see defaultUnifiedTextCheckerEnabled in Settings.cpp and Settings.in).
I'll provide a patch following the 2nd approach. Thanks for the feedback.
--
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