Bug 107684 - On Linux, can't get spelling suggestions without selection when unified text checker is enabled
Summary: On Linux, can't get spelling suggestions without selection when unified text ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Linux
: P2 Normal
Assignee: Grzegorz Czajkowski
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-23 05:55 PST by Grzegorz Czajkowski
Modified: 2013-01-31 10:01 PST (History)
7 users (show)

See Also:

Attachments
proposed patch (1.80 KB, patch)
2013-01-25 07:49 PST, Grzegorz Czajkowski 		rniwa: review+
Details | Formatted Diff | Diff
apply Ryosuke's suggestions (2.01 KB, patch)
2013-01-31 05:50 PST, Grzegorz Czajkowski 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Grzegorz Czajkowski 2013-01-23 05:55:59 PST 
Bug 103520 allows to retrieve the spelling suggestions without selecting the misspelled word.
We need to do the same when unified text checker is enabled.
Comment 1 Grzegorz Czajkowski 2013-01-25 07:49:19 PST 
Created attachment 184751 [details]
proposed patch
Comment 2 Ryosuke Niwa 2013-01-30 01:13:40 PST 
Comment on attachment 184751 [details]
proposed patch

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

> Source/WebCore/editing/Editor.cpp:1781
>          if (!range)
>              return Vector<String>();

I would prefer moving this to after if, and then called frame()->selection()->toNormalizedRange() in the else clause.

> Source/WebCore/editing/Editor.cpp:1783
> +            VisibleSelection wordSelection(frame()->selection()->base());

wordSelection = frame()->selection()->base() ?

> Source/WebCore/editing/Editor.cpp:1786
> +            ASSERT(range);

I don’t think we can assert that range is always non-null.
Comment 3 Grzegorz Czajkowski 2013-01-31 05:42:59 PST 
Comment on attachment 184751 [details]
proposed patch

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

>> Source/WebCore/editing/Editor.cpp:1781
>>              return Vector<String>();
> 
> I would prefer moving this to after if, and then called frame()->selection()->toNormalizedRange() in the else clause.

Done.

>> Source/WebCore/editing/Editor.cpp:1783
>> +            VisibleSelection wordSelection(frame()->selection()->base());
> 
> wordSelection = frame()->selection()->base() ?

I found two ways of constructing VisibleSelection object in WebCore: either
1) VisibleSelection wordSelection(positionObject) or
2) VisibleSelection wordSelection = VisibleSelection(positionObject).
I have updated this line with second one.

>> Source/WebCore/editing/Editor.cpp:1786
>> +            ASSERT(range);
> 
> I don’t think we can assert that range is always non-null.

Thanks for the hint. Done.
Comment 4 Grzegorz Czajkowski 2013-01-31 05:50:28 PST 
Created attachment 185765 [details]
apply Ryosuke's suggestions

Ryosuke, is it ok for you now? If yes feel free to land this patch.
I had to change the code a little to apply the review. I am not sure whether it can be landed without review.
Comment 5 Ryosuke Niwa 2013-01-31 09:48:44 PST 
Comment on attachment 185765 [details]
apply Ryosuke's suggestions

Yup, looks great to me.
Comment 6 WebKit Review Bot 2013-01-31 10:00:56 PST 
Comment on attachment 185765 [details]
apply Ryosuke's suggestions

Clearing flags on attachment: 185765

Committed r141429: <http://trac.webkit.org/changeset/141429>
Comment 7 WebKit Review Bot 2013-01-31 10:01:01 PST 
All reviewed patches have been landed.  Closing bug.