Our spell checking support is tied to the editor, so for the test case URL, you don't get the curly red lines until you do something which initializes the editor, like clicking on it. We need to eagerly initialize the editor for such input controls. Note to self: remove the autofocus attribute from test-input-spell in browser/base/content/test/subtst_contextmenu.html (added in bug 433860).

Wait. Why do we need to spell-check something that no one is editing? I've always considered that a bug in our spell-checking implementation....

That's a good question.

(In reply to comment #2) > Wait. Why do we need to spell-check something that no one is editing? I've > always considered that a bug in our spell-checking implementation.... Well, we don't _need to_. This is what we've been doing, so I didn't want to break that without a good reason. What do you suggest should happen when I focus a text field, edit something, don't know how to spell it, and then move to another field? Should we show the misspelling underlines after I move to another field as well? CCing Limi, since he might be interested in this from a UX standpoint.

> Should we show the misspelling underlines after I move to another field as well? That would be the easy thing to do if we keep the editor inited, right? I think disappearing the underlines would be very odd, personally. But I might be biased by implementation considerations. ;)

(In reply to comment #5) > > Should we show the misspelling underlines after I move to another field as well? > > That would be the easy thing to do if we keep the editor inited, right? Sure. But if that's expected, why isn't it expected to not have the underlines when the text control first shows up? This way, the underlines would suddenly appear when you focus the text control, which seems odd as well. > I think disappearing the underlines would be very odd, personally. But I might > be biased by implementation considerations. ;) I totally understand that! ;-)

> This way, the underlines would suddenly appear when you focus the text control, > which seems odd as well. To a first approximation (tabbing aside), focusing implies desiring to edit, right?

(In reply to comment #7) > > This way, the underlines would suddenly appear when you focus the text control, > > which seems odd as well. > > To a first approximation (tabbing aside), focusing implies desiring to edit, > right? Yes, sure. But what about the case where you move the focus to another control?

That's less obviously a desire to stop editing permanently...

OK. So, should we make textareas behave similarly as well? They will if we start initializing them lazily (I actually haven't worked on that, so I don't know how much work it's going to take, but we probably can't do this before bug 240933), but then again I'm not sure how much work it's going to take to do this for textareas if they're not lazily initialized. Then we have the issue of contenteditable fields, but I think those are different enough that we can want something different for them (like the existing behavior!).

CCing some UX folks here. I'd really like to get this patch out of my queue (either land it or remove it), so I'd appreciate the opinion of UX folks on what we should do here.

yay, I get to use our new shiny ux bugzilla keywords :) Only displaying the red lines when we initialize the editor violates ux-implementation-level, because the user doesn't know that the editor can be in a state when it is not initialized (or that it even exists). In terms of not spell checking until they click in as an intentional feature (irrespective of the initialization consideration), this could result in a ux-mode-error where the user doesn't realize that the application is in a special state, so they incorrectly form the conclusion that the text does not have any spelling errors, since no indicators are present. I recommend that we always display the spelling indicators.

As devil's advocate, why would a user expect us to be spell-checking text they can't edit, say (readonly fields come to mind)? Or text that they simply don't plan to edit? If we're going to seriously consider UI consistency of spell-checking behavior on the basis of what the user is going to assume, then I think we need some significant behavior changes anyway.

>why would a user expect us to be spell-checking text they >can't edit, say (readonly fields come to mind) They wouldn't and it would make sense to suppress the indicators (just as we don't spell check paragraphs of text, although that might be a fun extension). >Or text that they simply don't plan to edit? Unfortunately impossible to detect, but what are situations where the user encounters a lot of text that they do not plan to edit in a field? If they could see spelling errors, that might actually compel them to edit, making this case more complex. >I think we need some significant behavior changes anyway. what other cases do we need to consider?

(In reply to comment #14) > >why would a user expect us to be spell-checking text they > >can't edit, say (readonly fields come to mind) > > They wouldn't and it would make sense to suppress the indicators (just as we > don't spell check paragraphs of text, although that might be a fun extension). Hmm, we already don't show those indicators for readonly fields, as a matter of fact. Test case: data:text/html,<textarea>bazzinga!</textarea> data:text/html,<textarea readonly=true>bazzinga!</textarea>

(In reply to comment #12) > yay, I get to use our new shiny ux bugzilla keywords :) Heh, you were so excited that you actually forgot to use the keywords! ;-)

> but what are situations where the user encounters a lot of text that they do > not plan to edit in a field? Pretty much any time there are multiple text fields on a page... > Hmm, we already don't show those indicators for readonly fields Do we also skip running the actual spellcheck? If so, great.

(In reply to comment #17) > > Hmm, we already don't show those indicators for readonly fields > > Do we also skip running the actual spellcheck? If so, great. Yes. We don't spell check read-only fields.

Bug 364914 should probably share whatever fate befalls this bug.

OK, I guess this is ready for roc's review now.

Comment on attachment 448820 [details] [diff] [review] Patch (v1) a=me, on the condition that any regressions cause an immediate backout, this seems riskier than normal