| Summary: | On Linux, can't get spelling suggestions without selection when unified text checker is enabled | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Grzegorz Czajkowski <g.czajkowski> | ||||||
| Component: | WebKit EFL | Assignee: | Grzegorz Czajkowski <g.czajkowski> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | laszlo.gombos, lucas.de.marchi, mifenton, morrita, rniwa, tony, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | Linux | ||||||||
| Attachments: |
|
||||||||
|
Description
Grzegorz Czajkowski
2013-01-23 05:55:59 PST
Created attachment 184751 [details]
proposed patch
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 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. 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 on attachment 185765 [details]
apply Ryosuke's suggestions
Yup, looks great to me.
Comment on attachment 185765 [details] apply Ryosuke's suggestions Clearing flags on attachment: 185765 Committed r141429: <http://trac.webkit.org/changeset/141429> All reviewed patches have been landed. Closing bug. |