Bug 80136 - [EFL][DRT] Implement LayoutTestController's methods related with editing
Summary: [EFL][DRT] Implement LayoutTestController's methods related with editing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-02 02:52 PST by Grzegorz Czajkowski
Modified: 2012-03-23 00:25 PDT (History)
5 users (show)

See Also:


Attachments
proposed patch (14.66 KB, patch)
2012-03-05 07:00 PST, Grzegorz Czajkowski
no flags Details | Formatted Diff | Diff
updated patch according to Kubo's review (12.86 KB, patch)
2012-03-07 00:58 PST, Grzegorz Czajkowski
no flags Details | Formatted Diff | Diff
rebased patch to HEAD (12.83 KB, patch)
2012-03-07 01:37 PST, Grzegorz Czajkowski
rniwa: review-
rniwa: commit-queue-
Details | Formatted Diff | Diff
updated patch (11.20 KB, patch)
2012-03-22 07:16 PDT, Grzegorz Czajkowski
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Grzegorz Czajkowski 2012-03-02 02:52:22 PST
Implements EFL's LayoutTestController methods:
 - setSmartInsertDeleteEnabled,
 - setSelectTrailingWhitespaceEnabled
 - hasSpellingMarker.

I am working on this implementation, patch will be uploaded soon.
Comment 1 Grzegorz Czajkowski 2012-03-05 07:00:42 PST
Created attachment 130122 [details]
proposed patch
Comment 2 Raphael Kubo da Costa (:rakuco) 2012-03-05 08:18:39 PST
Comment on attachment 130122 [details]
proposed patch

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

I'd rather avoid doing what the Qt port does/used to do and make WebCoreSupport aware of DRT; the current code was written with that in mind, and I think we can keep doing so. Isn't it possible to make DRTSupportEefl::setSelectTrailingWhitespaceEnabled set some flag in EditClient itself or in ewk_view?

> LayoutTests/platform/efl/Skipped:67
> +editing/selection/doubleclick-inline-first-last-contenteditable.html

Please keep the list alphabetically sorted.
Comment 3 Grzegorz Czajkowski 2012-03-07 00:58:13 PST
Created attachment 130563 [details]
updated patch according to Kubo's review
Comment 4 Grzegorz Czajkowski 2012-03-07 01:00:14 PST
(In reply to comment #2)
> (From update of attachment 130122 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130122&action=review
> 
> I'd rather avoid doing what the Qt port does/used to do and make WebCoreSupport aware of DRT; the current code was written with that in mind, and I think we can keep doing so. Isn't it possible to make DRTSupportEefl::setSelectTrailingWhitespaceEnabled set some flag in EditClient itself or in ewk_view?

Ok, I added flag to EditorClinetEfl to set/get selectTrailingWhitespaceEnabled (similar to smartInsertDeleteEnabled).

> 
> > LayoutTests/platform/efl/Skipped:67
> > +editing/selection/doubleclick-inline-first-last-contenteditable.html
> 
> Please keep the list alphabetically sorted.

Done.
Comment 5 Grzegorz Czajkowski 2012-03-07 01:37:02 PST
Created attachment 130568 [details]
rebased patch to HEAD
Comment 6 Raphael Kubo da Costa (:rakuco) 2012-03-07 03:56:55 PST
Comment on attachment 130568 [details]
rebased patch to HEAD

Looks fine to me.
Comment 7 Ryosuke Niwa 2012-03-08 00:06:24 PST
Comment on attachment 130568 [details]
rebased patch to HEAD

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

> Source/WebKit/efl/WebCoreSupport/EditorClientEfl.cpp:235
> +    m_smartInsertDeleteEnabled = enabled;

You also need to set m_selectTrailingWhitespaceEnabled false if enabled was true here.

> Source/WebKit/efl/WebCoreSupport/EditorClientEfl.cpp:245
> +    m_selectTrailingWhitespaceEnabled = enabled;

You need to set m_smartInsertDeleteEnabled false if enabled was true here.
Comment 8 Grzegorz Czajkowski 2012-03-22 07:16:42 PDT
Created attachment 133254 [details]
updated patch

Added Ryosuke's suggestions.
Removed hasSpellingMarker implementation from current patch - hasSpellingMarker should be used through Internals interface https://bugs.webkit.org/show_bug.cgi?id=81300.

Patch for landing :)
Comment 9 WebKit Review Bot 2012-03-23 00:25:43 PDT
Comment on attachment 133254 [details]
updated patch

Clearing flags on attachment: 133254

Committed r111835: <http://trac.webkit.org/changeset/111835>
Comment 10 WebKit Review Bot 2012-03-23 00:25:48 PDT
All reviewed patches have been landed.  Closing bug.