Bug 89331

Summary: [chromium] Spellchecker should show suggestions only when right-clicking a misspelled word.
Product: WebKit Reporter: Hironori Bono <hbono>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: morrita, progame+wk, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 89444    
Bug Blocks:    
Attachments:
Description Flags
Patch v1
morrita: review+
Patch v1 (updated ChangeLog)
none
Patch v2 (fixed a regression on Mac) none

Description Hironori Bono 2012-06-17 23:53:35 PDT
Greetings,

Step to reproduce:
1. Type "This is a lasy doga." in a text area.
2. Type ctrl+A to select all text in the text area.
3. Right-click a mouse.

Expected Behavior:
* Chromium should not show suggestions.

Actual Behavior:
* Chromium shows two suggestions: "last" and "dog".

Unfortunately, my r118740 <http://trac.webkit.org/changeset/118740> has a bug that Chrome may show suggestion for all misspelled words in the selection if it is not collapsed. Also, my r120274 <http://trac.webkit.org/changeset/120274> has a bonehead mistake that used a wrong range to replace text.

Regards,

Hironori Bono
Comment 1 Hironori Bono 2012-06-18 00:02:44 PDT
Created attachment 148065 [details]
Patch v1

Greetings Morita-san,

Would it be possible to review this quick fix?

Regards,

Hironori Bono
Comment 2 Hajime Morrita 2012-06-18 00:45:17 PDT
Comment on attachment 148065 [details]
Patch v1

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

The code looks good.

> Source/WebKit/chromium/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=89331

Could you give a bit more detailed explanation?
Seeing only this change doesn't give any rationale behind the change.
Comment 3 Hironori Bono 2012-06-18 01:21:27 PDT
Created attachment 148070 [details]
Patch v1 (updated ChangeLog)

Greetings Morita-san,

Many thanks for your quick review.
I have updated ChangeLog to describe the background.

Regards,

Hironori Bono
Comment 4 WebKit Review Bot 2012-06-18 02:32:31 PDT
Comment on attachment 148070 [details]
Patch v1 (updated ChangeLog)

Rejecting attachment 148070 [details] from review queue.

hbono@chromium.org does not have reviewer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have reviewer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your reviewer rights.
Comment 5 WebKit Review Bot 2012-06-18 04:09:51 PDT
Comment on attachment 148070 [details]
Patch v1 (updated ChangeLog)

Clearing flags on attachment: 148070

Committed r120586: <http://trac.webkit.org/changeset/120586>
Comment 6 WebKit Review Bot 2012-06-18 04:09:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Hironori Bono 2012-06-18 22:40:39 PDT
Greetings,

Sorry, I noticed this was not a good fix. (It is better for us to emulate Firefox.) Would it be possible to roll out my r120586?

Regards,

Hironori Bono
Comment 8 WebKit Review Bot 2012-06-19 01:07:41 PDT
Re-opened since this is blocked by 89444
Comment 9 Hironori Bono 2012-06-19 03:37:55 PDT
Created attachment 148305 [details]
Patch v2 (fixed a regression on Mac)

Greetings Morita-san,

Thanks for your help and apologies for my previous change. I have re-implemented my change to emulate the behavior of Mac Chrome so we do not need workarounds for Mac. Would it be possible to review this change?

Regards,

Hironori Bono
Comment 10 WebKit Review Bot 2012-06-20 00:00:55 PDT
Comment on attachment 148305 [details]
Patch v2 (fixed a regression on Mac)

Clearing flags on attachment: 148305

Committed r120810: <http://trac.webkit.org/changeset/120810>
Comment 11 WebKit Review Bot 2012-06-20 00:01:03 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Yair Yogev 2012-08-15 17:22:03 PDT
This fix is the cause of a very weird regression in Windows (only tested windows):
let's say i'm posting a comment IGNORETHISWORD in WebKit Bugzilla, just like i am right now .
All my words are spelled correctly except for one, that capital letters gibberish in the second line.

Now i would like to copy my comment before submitting, so i: 
1. Press Ctrl+A
2. right click the selection (anywhere)

The selection will now change: The part of the text before the gibberish word will be deselected and if the last text block is longer than the horizontal position of the gibberish word, then any text in that block after that position will also be deselected.
Comment 13 Yair Yogev 2012-08-15 17:22:30 PDT
screenshot of comment #12 http://i47.tinypic.com/2myrzpf.jpg
Comment 14 Yair Yogev 2012-08-15 17:24:38 PDT
Was this change suppose to actually change the selection? i don't think it should...
Comment 15 Yair Yogev 2012-08-15 17:30:40 PDT
For one lined text field, with one misspelled word, it deselects the whole selection except for the misspelled word, although i didn't right click the misspelled word and all i wanted to do is Copy the text using the context menu option...

I hope that's not intentional, the spell checker shouldn't interfere with basic text handling capabilities.