Bug 14552

Summary: Add a way to disable spell checking for specific element
Product: WebKit Reporter: Sergei Yakovlev <sergei.yakovlev>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Enhancement CC: apeller, ap, arosenzweig, dev+webkit, dwood, eric, ernst, fuckingregistration2, jl, joshm, jparent, mike, ojan, rpaplin, tom, tony, webkit
Priority: P4 Keywords: InRadar
Version: 523.x (Safari 3)   
Hardware: All   
OS: OS X 10.4   
Bug Depends on:    
Bug Blocks: 9915, 25537, 25536, 25539    
Attachments:
Description Flags
A quick fix (without unit-tests)
none
proposed fix with a layout test
eric: review+
Screenshot of spellcheck attribute not being effective in latest Chrome none

Description Sergei Yakovlev 2007-07-07 10:43:29 PDT
Spell checking is a global setting in Safari. All text fields, text areas and contenteditable elements are either spell-checked or not. This is great because it's simple. However, there might be a need to disable spell checking on specific elements. (For example, a text field where user enters several space-delimited logins.) Right now, there's no way to do that in Safari.

In Firefox, this is achieved by setting spellcheck="false" on an element. For Safari implementation, it might be better to add a private CSS property, something like -khtml-spell-checking: disable.
Comment 1 webkit 2007-09-30 01:47:59 PDT
*** Bug 15095 has been marked as a duplicate of this bug. ***
Comment 2 webkit 2007-09-30 01:53:12 PDT
On text editors, like FCKeditor, users have no access to the browser default context menu, as the editor customizes it to its needs. So, while editing a document, users are presented with the red-underline spell check mark for misspelled words, but have no ways to access the context menu for replacements.

Many users and developers prefer to not have the spell checker in this way, so we make it configurable in the editor. For Firefox, we simply set <body spellcheck="false">.
Comment 3 Vicki Murley 2008-04-21 17:32:03 PDT
<rdar://problem/5879133>
Comment 4 Aaron Rosenzweig 2008-04-28 12:37:13 PDT
I haven't seen much chatter since 2007 and Vicki's more recent comment relating this issue to Radar issue #5879133 doesn't seem to be viewable by the general public as far as I can tell. Could someone please comment on the status of this issue? It would be nice to programmatically disable spell checking on various HTML text fields. 
Comment 5 webkit 2008-04-29 01:34:07 PDT
While adding this option for every single element may be challenging, I would definitely recommend providing this feature in two steps. At first, make it possible to disable spell checking on input areas as a whole. This should be relatively simple, and would probably embrace all real world usage cases for it.

So, at first, this new attribute/css property would be valid for <input>, <textarea> and <body> only.

After that, if we feel it is necessary, this attribute could have effect in all elements, but I don't see much usage for it for specific elements only in a document, like inside an <em> for example. So, I would not give much priority for this case.
Comment 6 Aaron Rosenzweig 2008-04-29 07:00:25 PDT
+1 for Frederico's comments.

If we could have a CSS property that we could use for textArea and input tags that would be great.
Comment 7 Robbie Paplin 2008-04-29 07:28:38 PDT
Don't forget to allow contenteditable elements (<div> is most common) to set/alter/respect this setting as well.

Common use cases include
* Recipient wells in web mail applications
* User Name / Logon forms
* Fields that tend to have unfamiliar words (names, places, addresses, etc)
* Applications that already have server side spell checking (and wish to remain consistent across browsers / platforms)

Here's the Firefox documentation: http://developer.mozilla.org/en/docs/Controlling_spell_checking_in_HTML_forms

I'd prefer it if Safari used the spellcheck="false" attribute, since application developers are already familiar with it and web applications already use it. However, I'm not opposed to a private CSS property, in case others feel it's a more appropriate solution.
Comment 8 Aaron Rosenzweig 2008-04-29 10:19:55 PDT
Robbie's point is well taken.

It is important to have both methods:

1) spellcheck="false" - an element attribute

2) -khtml-spell-checking: disable - a CSS property

Regarding #1 - the upside is that it is already known and works in Firefox and is also easy to use. Existing code that is tweaked for Firefox will behave appropriately in Safari. The downside is you have to modify your app code in many places to make sure that your text fields tack on the attribute in the HTTP response to the browser. It's not easy to globally make sure all text fields behave consistently.

Regarding #2 - the upside is that a few lines in a CSS file can give global preference modifications across your whole web app and also give you the ability for fine grained control on a page level or even as minute as a single text input. You don't even have to modify your web app code to achieve these effects. If you so choose, at the programming level, you could return specific information in the "style" attribute to achieve something like in #1. The only downside is that this CSS property is not currently in use so there is no precedent for it.
Comment 9 Matt Lilek 2008-04-29 10:47:11 PDT
Not that my $0.02 really count for anything, but it doesn't really make sense to me to have a CSS property that controls whether or not an element is spell checked.

Spell checking is a behavior of the control, not a presentational aspect.  If there's a CSS property to control this, then why not add one for contenteditable?  I can't think of any CSS property that controls an actual behavior of the element it is applied to.

A property that allows you to override the appearance of misspelled words sounds fine, but as for actually controlling whether it spell checks or not doesn't really make sense to me - again, just my two cents.
Comment 10 Sergei Yakovlev 2008-04-29 12:01:59 PDT
Actually, there *is* a CSS property for contenteditable:

    -webkit-user-modify: read-write;

So it makes sense to add a CSS property for spell checking. I would suggest naming it "spell-check" (like "border-collapse", "user-modify" etc.). Example:

    -webkit-spell-check: none;
    -webkit-spell-check: check;
Comment 11 Sergei Yakovlev 2008-04-29 12:10:58 PDT
Regarding the spellcheck="false" attribute, here's yet another reason to have it: source code. Example (taken from CouchDB):

<div id="tempview">
  <div class="top"><label>View function</label></div>
  <textarea rows="8" cols="79" spellcheck="false">function(doc) {
  map(null, doc);
}</textarea>
  <div class="bottom"><button type="button">Run</button></div>
</div>
Comment 12 Matt Lilek 2008-04-29 12:38:31 PDT
(In reply to comment #10)
> Actually, there *is* a CSS property for contenteditable:
> 
>     -webkit-user-modify: read-write;
> 

Well then, my bad - don't know how I missed that. Still seems kind of silly to me (from more of a web dev side than a browser side) for that to be controlled by CSS. *shrug*
Comment 13 Dave Hyatt 2008-04-29 12:45:42 PDT
I don't think CSS is appropriate for this.  (CSS was not really appropriate as the way of turning editability on and off either, and I'd like to fix that eventually also.)  An attribute is better.
Comment 14 Johan "Spocke" Sörlin 2008-05-14 04:16:59 PDT
It would be ideal to implement the FF way of setting the spellcheck attribute of the body as suggested. I think this bug is important since some of our users doesn't understand that Safari is the one adding these elements and blame our application.

It would also be of use to change the appearance of the spellchecker lines since you might want to use it on red surface but then it should be some css like -webkit-spellchecker-color.
Comment 15 Peter Kasting 2008-08-19 11:01:15 PDT
FWIW, the draft spellcheck attribute spec (which should match what I implemented in Firefox 2 pretty closely) is currently sitting around at http://damowmow.com/playground/spellcheck.txt .
Comment 16 Hironori Bono 2009-01-06 00:30:44 PST
Created attachment 26453 [details]
A quick fix (without unit-tests)

Even though I implemented a quick fix for this bug, I'm wondering how I should write unit tests for this fix. (This is the reason why I don't add the "review" flag.) Is it possible to give me suggestions about how I should test this fix?

By the way, this fix just adds code to ascend a DOM tree to find a "spellcheck" attribute and use its value to enable (or disable) its spellchecker.
Comment 17 Alexey Proskuryakov 2009-02-11 23:12:39 PST
Seems that pixel results will need to be used, as in other spelling tests in editing/spelling.
Comment 18 Alexey Proskuryakov 2009-02-11 23:14:55 PST
Hixie says: <http://damowmow.com/playground/spellcheck.txt>.
Comment 19 Hironori Bono 2009-02-20 00:38:05 PST
Created attachment 27825 [details]
proposed fix with a layout test

Thank you for your comments and sorry for my slow update.
I fixed some code-style problems and added a layout test which tests the "spellcheck" attribute can enable (or disable) a spell-checker.
Comment 20 Eric Seidel (no email) 2009-02-20 00:52:25 PST
I'm surprised Dave is anti-CSS for this.  We use CSS for everything else in the engine. :)  It seems that if we want to implement this as something inherited, it should probably just be hung off of RenderStyle like any CSS property would be.  I'd be interested in in Dave's comments as to why that would be a bad idea.  Making it CSS also makes it easy for us to turn it off for input[type=password] in html4.css too :)
Comment 21 Johan "Spocke" Sörlin 2009-03-31 09:15:06 PDT
I'm bumping this one since the "spellcheck" attribute is now part of the HTML 5 spec and it's also implemented by other vendors. I hope to see this feature in the Safari 4 final.
Comment 22 Dion Almaer 2009-04-13 17:46:13 PDT
In some ways it is a behaviour for the control, but in another, it allows you to "get rid of the squiggles" which is very much a style thing :)
Comment 23 Ernst de Haan 2009-04-13 22:41:39 PDT
There is -in my opinion- even a strong case for considering this a display issue only, and not behaviour: in fact a setting could be interpreted to say: "I don't care if you do any spell checking, kust as long as you do not display the results."

A typical production-quality implementation then also disables the actual behaviour, for performance reasons.
Comment 24 Eric Seidel (no email) 2009-05-03 22:11:09 PDT
Comment on attachment 27825 [details]
proposed fix with a layout test

Ok, this looks fine.

I don't think you want to have the editing callbacks on (because they don't seem to dump anything useful here).

I will find some way to turn off the editing callbacks and land this.
Comment 25 Eric Seidel (no email) 2009-05-03 22:16:39 PDT
Comment on attachment 27825 [details]
proposed fix with a layout test

Actually, I'm looking at the spec, and I don't think this is quite a full implementation:
http://www.whatwg.org/specs/web-apps/current-work/#attr-spellcheck
Comment 26 Eric Seidel (no email) 2009-05-03 22:26:31 PDT
All we're missing is the javascript accessor, which can be done in a separate change.

Also, the spec seems not quite fully finished here.  I read it through once, and I'm still unclear as to which elements have "true by default" as their state.

Also, steps 4 and 7 seem contradictory:
4.  Otherwise, if there is an ancestor element with a spellcheck content attribute that is not in the inherit state, then: if the nearest such ancestor's spellcheck content attribute is in the true state, then checking is enabled; otherwise, checking is disabled.
7.  Otherwise, if the element's parent element has its checking enabled, then checking is enabled.

End result: this patch is fine.  It brings us closer to FF and the eventual spec behavior.
Comment 27 Ojan Vafai 2009-05-03 22:55:44 PDT
A simple addition to this that might be worth considering is that spellcheck='' should turn it on. That can totally be a followup bug/patch though.
Comment 28 Justin Garcia 2009-05-03 23:38:08 PDT
Spec says:

"For text that is part of text or CDATA nodes, the element with which the text is associated is the element that is the immediate parent of the start of the word, sentence, or other piece of text. For text in attributes, it is the element with which the attribute is associated. For text in text fields, it is the relevant input or textarea element."

But the patch does:

+    // Ascend the DOM tree to find a "spellcheck" attribute.
+    // When we find a "spellcheck" attribute, retrieve its value and exit if its value is "false".
+    const Node* node = editor->frame()->document()->focusedNode();
+    while (node) {

So, for example, the testcase mentioned in the spec:

<div contenteditable="true"><span spellcheck="false" id="a">Hell</span><em>o!</em></div>

Wouldn't work.
Comment 29 Eric Seidel (no email) 2009-05-03 23:57:13 PDT
(In reply to comment #28)
> So, for example, the testcase mentioned in the spec:
> 
> <div contenteditable="true"><span spellcheck="false"
> id="a">Hell</span><em>o!</em></div>
> 
> Wouldn't work.

Agreed.  I've filed a follow-up bug with test case:
https://bugs.webkit.org/show_bug.cgi?id=25537
Comment 30 Eric Seidel (no email) 2009-05-03 23:57:48 PDT
I think this new behavior is better than nothing though, so I plan to leave the patch in, unless you disagree.
Comment 31 Ojan Vafai 2009-05-04 00:09:03 PDT
Filed followup https://bugs.webkit.org/show_bug.cgi?id=25539
Comment 32 Julie Parent 2009-05-07 21:31:19 PDT
Filled another followup bug: https://bugs.webkit.org/post_bug.cgi

This only disables the red squiggle, not the context menu.
Comment 33 Tony Chang 2010-05-12 00:55:34 PDT
(In reply to comment #32)
> Filled another followup bug: https://bugs.webkit.org/post_bug.cgi
> 
> This only disables the red squiggle, not the context menu.

The follow up bug is https://bugs.webkit.org/show_bug.cgi?id=25639
Comment 34 fuckingregistration2 2013-03-06 10:59:49 PST
What's up with this bug its status is "Resolved fixed" however in current version of Chrome the spellcheck attribute does not seem to be effective at all.

Anything I'm missing?
Comment 35 fuckingregistration2 2013-03-06 11:08:42 PST
Created attachment 191797 [details]
Screenshot of spellcheck attribute not being effective in latest Chrome
Comment 36 fuckingregistration2 2013-03-06 11:09:34 PST
Simple test: http://jsfiddle.net/fAM7E/