Bug 89331 - [chromium] Spellchecker should show suggestions only when right-clicking a misspelled word.
Summary: [chromium] Spellchecker should show suggestions only when right-clicking a mi...
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on: 89444
  Show dependency treegraph
Reported: 2012-06-17 23:53 PDT by Hironori Bono
Modified: 2012-08-15 17:30 PDT (History)
3 users (show)

See Also:

Patch v1 (3.28 KB, patch)
2012-06-18 00:02 PDT, Hironori Bono
morrita: review+
Details | Formatted Diff | Diff
Patch v1 (updated ChangeLog) (3.63 KB, patch)
2012-06-18 01:21 PDT, Hironori Bono
no flags Details | Formatted Diff | Diff
Patch v2 (fixed a regression on Mac) (4.16 KB, patch)
2012-06-19 03:37 PDT, Hironori Bono
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hironori Bono 2012-06-17 23:53:35 PDT

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.


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?


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.


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

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?


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?


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.