Bug 39402

Summary: [chromium] disable spelling suggestions if spellcheck=false
Product: WebKit Reporter: Tony Chang <tony>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ojan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch tkent: review+

Tony Chang
Reported 2010-05-19 21:31:43 PDT
[chromium] disable spelling suggestions if spellcheck=false
Attachments
Patch (1.73 KB, patch)
2010-05-19 21:32 PDT, Tony Chang 		tkent: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Tony Chang
Comment 1 2010-05-19 21:32:58 PDT
Created attachment 56558 [details] Patch
Kent Tamura
Comment 2 2010-05-19 22:19:41 PDT
Comment on attachment 56558 [details] Patch WebKit/chromium/src/ContextMenuClientImpl.cpp:205 + data.isSpellCheckingEnabled = true; Should we leave this line in a case of no misspelledWord?
Tony Chang
Comment 3 2010-05-19 22:28:16 PDT
(In reply to comment #2) > (From update of attachment 56558 [details]) > WebKit/chromium/src/ContextMenuClientImpl.cpp:205 > + data.isSpellCheckingEnabled = true; > Should we leave this line in a case of no misspelledWord? I left it in to be consistent with Safari. This means that the context menu will still show a check mark by "Spellchecking in this field". I did it this way in Safari because you can imagine a div with spellchecking enabled, with a single span inside it that has spellchecking disabled. It seems more consistent to treat the whole field as "spellcheck enabled" even if you click in the span. I think it would be confusing if the check mark changed depending on which node you happened to click on within a single content editable node.
Kent Tamura
Comment 4 2010-05-19 22:31:27 PDT
Comment on attachment 56558 [details] Patch (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 56558 [details] [details]) > > WebKit/chromium/src/ContextMenuClientImpl.cpp:205 > > + data.isSpellCheckingEnabled = true; > > Should we leave this line in a case of no misspelledWord? > > I left it in to be consistent with Safari. This means that the context menu will still show a check mark by "Spellchecking in this field". I did it this way in Safari because you can imagine a div with spellchecking enabled, with a single span inside it that has spellchecking disabled. It seems more consistent to treat the whole field as "spellcheck enabled" even if you click in the span. I think it would be confusing if the check mark changed depending on which node you happened to click on within a single content editable node. Ok, that's reasonable.
Tony Chang
Comment 5 2010-05-19 22:35:44 PDT
Committed r59819: <http://trac.webkit.org/changeset/59819>
Note You need to log in before you can comment on or make changes to this bug.