Bug 9887 - REGRESSION: WebContinuousSpellCheckingEnabled should enable continuous spell checking on all text areas
Summary: REGRESSION: WebContinuousSpellCheckingEnabled should enable continuous spell ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Nobody
URL:
Keywords: InRadar, Regression
: 9884 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-07-12 21:27 PDT by David Kilzer (:ddkilzer)
Modified: 2007-01-29 12:59 PST (History)
7 users (show)

See Also:


Attachments
Proposed Patch (1.95 KB, patch)
2006-09-12 19:46 PDT, Vladimir Olexa (vladinecko)
timothy: review-
Details | Formatted Diff | Diff
Patch Revised (2.74 KB, patch)
2006-09-26 22:02 PDT, Vladimir Olexa (vladinecko)
timothy: review-
Details | Formatted Diff | Diff
Patch (3.43 KB, patch)
2006-09-27 11:53 PDT, Vladimir Olexa (vladinecko)
timothy: review-
Details | Formatted Diff | Diff
Yet another Patch (3.89 KB, patch)
2006-09-27 13:20 PDT, Vladimir Olexa (vladinecko)
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2006-07-12 21:27:26 PDT
Setting the WebContinuousSpellCheckingEnabled default to 1 should enable continuous spell checking on all text fields and text areas in WebKit without having to change the setting through the contextual menu.

The current production Safari 2.0.4 (419.3) on Mac OS X 10.4.7 (8J135/PowerPC) does this for text areas, but not text fields.  On a locally-built WebKit r15401, neither text fields nor text areas have continuous spell checking on if this default is set.
Comment 1 Rosyna 2006-07-13 11:35:54 PDT
Just to note, I do not believe this should be on for text fields by default. But it should be on for text areas by default. The former mimics the rest of Mac OS X.
Comment 2 Alice Liu 2006-07-14 17:06:11 PDT
<rdar://problem/4631821>
Comment 3 David Kilzer (:ddkilzer) 2006-07-15 12:47:30 PDT
Changing title of bug and removed NativeTextField keyword per Comment #1.  Production Safari only has continuous spell checking on text areas when WebContinuousSpellCheckingEnabled is set, not text fields.

Comment 4 Vladimir Olexa (vladinecko) 2006-09-12 19:46:40 PDT
Created attachment 10520 [details]
Proposed Patch

Although, it has been mentioned this should be specific to TextAreas only, this patch makes changes globally. Meaning, if you change enable/disable spell checking on a text area, it will also enable/disable it on text fields. However, it does remember its state until it's changed again :)

After discussion with Adele, maybe a separate bug can be filed (if it is, in fact, a bug) to make the setting separate from text fields.
Comment 5 Vladimir Olexa (vladinecko) 2006-09-12 21:36:55 PDT
To me, having the text field and text area set separately seems more like a bug than a feature. When I first enabled the spell checker in shipping Safari on a text area and then later typed something in a field (e.g. subject line in an email client) I was very surprised the typed text wasn't checked automatically and that I was forced to enable it again. 

What do you guys think?
Comment 6 John Sullivan 2006-09-13 05:53:39 PDT
For Safari's sake, I think a single setting to control both text fields and textareas would be best. I don't know if other WebKit clients might want to control these separately.

Comment 7 Scott Menor 2006-09-16 14:05:41 PDT
I agree with John and  Vladimir - if selected, all editable text fields should be spellchecked and the setting should be persistent (it's frustrating to have to re-enable spellcheck for every field). 

It seems bizarre to think that someone would want to spellcheck textareas but not text input fields. Then again, since it doesn't do anything more than underline misspelled words without user interaction, it seems bizarre to me that anyone wouldn't want continuous spellcheck enabled all the time.
Comment 8 Timothy Hatcher 2006-09-25 22:24:29 PDT
Comment on attachment 10520 [details]
Proposed Patch

isContinuousSpellCheckingEnabled is semi hot code, so it would be best to just have a file static BOOL continuousSpellCheckingEnabled. This will be faster than a lookup through NSUserDefaults.

Also we usually don't store these toggled settings in a NSUserDefault, the application would normally do this and call setContinuousSpellCheckingEnabled with the default value. We don't have a default for setSmartInsertDeleteEnabled, etc. If we did add a new NSUserDefaults key, it should be added to WebPreferenceKeysPrivate.h.
Comment 9 Vladimir Olexa (vladinecko) 2006-09-26 22:02:39 PDT
Created attachment 10792 [details]
Patch Revised

As discussed on IRC, just minor changes were made to this patch (only read from NSUserDefaults once per session) as shipping Safari does use WebContinuousSpellCheckingEnabled.
Comment 10 Timothy Hatcher 2006-09-27 10:48:42 PDT
Comment on attachment 10792 [details]
Patch Revised

This looks good. 

We still should add WebContinuousSpellCheckingEnabled as a define in WebPreferenceKeysPrivate.h.
Comment 11 Vladimir Olexa (vladinecko) 2006-09-27 11:53:31 PDT
Created attachment 10801 [details]
Patch

added a define in WebPreferencesPrivate.h
Comment 12 Timothy Hatcher 2006-09-27 11:58:22 PDT
Comment on attachment 10801 [details]
Patch

Sorry. I meant WebPreferenceKeysPrivate.h. You will see other keys in there.
Comment 13 Vladimir Olexa (vladinecko) 2006-09-27 13:20:11 PDT
Created attachment 10804 [details]
Yet another Patch

define moved to WebPrefenceKeysPrivate.h
Comment 14 Mark Rowe (bdash) 2006-10-05 14:58:04 PDT
Vladimir, can you please be sure to double-check your patches for tabs in future?

Landed in r16823.
Comment 15 Jon 2006-10-05 22:00:33 PDT
Patch for this bug doesn't reproduce the previous behavior. Before, only TextArea's always obeyed the spell checking preference. TextFields did not. And having spell checking active for TextFields is really something situational, since you type actual words in TextFields much less often than TextAreas.
Comment 16 Vladimir Olexa (vladinecko) 2006-10-06 06:30:21 PDT
Jon,

as you read the previous comments you'll see we were trying to figure out what the default behaviour should be. At the end, I think most people agreed that to have text areas and text fields set separately didn't make much sense and is kind of confusing for the user. Once you set it for text areas and you know it's working there, you go type something in the subject field (for example) and all of the sudden, nothing gets checked. So you either assume you've spelled everything correctly, or that the spell checker is not available/implemented for text fields, both of which may not be true.

And regarding typing words in text fields much less than in text areas, I agree you type fewer words in fields than in areas, but they're words, nonetheless.
Comment 17 John Sullivan 2006-10-06 07:03:13 PDT
There might be more cases where a one-line text field might be intended to hold non-words than a multi-line textarea, but we decided that it makes more sense for the setting to apply to both.
Comment 18 John Sullivan 2007-01-29 12:59:03 PST
*** Bug 9884 has been marked as a duplicate of this bug. ***