RESOLVED FIXED 107684
On Linux, can't get spelling suggestions without selection when unified text checker is enabled
https://bugs.webkit.org/show_bug.cgi?id=107684
Summary On Linux, can't get spelling suggestions without selection when unified text ...
Grzegorz Czajkowski
Reported 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.
Attachments
proposed patch (1.80 KB, patch)
2013-01-25 07:49 PST, Grzegorz Czajkowski
rniwa: review+
apply Ryosuke's suggestions (2.01 KB, patch)
2013-01-31 05:50 PST, Grzegorz Czajkowski
no flags
Grzegorz Czajkowski
Comment 1 2013-01-25 07:49:19 PST
Created attachment 184751 [details] proposed patch
Ryosuke Niwa
Comment 2 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.
Grzegorz Czajkowski
Comment 3 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.
Grzegorz Czajkowski
Comment 4 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.
Ryosuke Niwa
Comment 5 2013-01-31 09:48:44 PST
Comment on attachment 185765 [details] apply Ryosuke's suggestions Yup, looks great to me.
WebKit Review Bot
Comment 6 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>
WebKit Review Bot
Comment 7 2013-01-31 10:01:01 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.