737889 - Spell checking is broken when typing Facebook messages

Status: RESOLVED FIXED

(Core :: Spelling checker, defect)

Description

[(no longer active)]

This is similar to bug 734530, but is not fixed by the patch there. :(

Comment 1

[Timothy Nikkel (:tnikkel)]

Doesn't seem to be due to a recent regression in Firefox. I tried builds back to 2008 and none of them underlined misspelled words. Might be due to a more recent change to Facebook's website though, I don't know.

When I get the "popup" 'New Message' box that looks like its on top of the page contents when you send a message to someone you haven't sent a message to before the spelling checker does seem to work. Just when you get a reply box in an existing "conversation" is a problem for me.

Comment 2

[(no longer active)]

OK I have a test case now.  There are actually two bugs involved here, bug 734530 and this bug.  We basically don't update the editable state of the anonymous children of the textbox when its readonly attribute is removed, and the spell checker has code to make sure it would never check non-editable content, which causes the textbox to not be spell checked.

This is indeed not a recent regression.

Comment 3

[(no longer active)]

Attachment: Patch (v1)

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 608125 [details] [diff] [review]
Patch (v1)

Review of attachment 608125 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/nsHTMLTextAreaElement.cpp
@@ +1296,5 @@
> +      mState->UpdateEditableState(aNotify);
> +      nsTextEditorState *state = GetEditorState();
> +      if (state) {
> +        state->UpdateEditableState(aNotify);
> +      }

I don't understand why you need to update GetEditorState(). Isn't mState the only state we need to worry about for textareas?

(And wouldn't it make sense for GetEditorState() to actually return mState for textareas?)

Comment 5

[Mozilla RelEng Bot]

Try run for 063db6b952f4 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=063db6b952f4
Results (out of 14 total builds):
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-063db6b952f4

Comment 6

[(no longer active)]

Attachment: Patch (v1)

Sorry. that was the wrong patch!

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 608391 [details] [diff] [review]
Patch (v1)

Review of attachment 608391 [details] [diff] [review]:
-----------------------------------------------------------------

That makes more sense :-)

Comment 8

[(no longer active)]

Attachment: Part 2

This patch triggered a test failure which seems to be a bug that we've had for ever.  If you drop something over a non-editable content on a page with an editor, we fail to stop the event propagation, which means that the code in contentAreaDropListener.js kicks in and tries to do a link navigation (among other things).  For example, if the dropped text is "foo bar", we end up trying to navigate to http://foo%20bar which results in an error page.

This patch fixes that problem.

Comment 9

[Mozilla RelEng Bot]

Try run for c223dd45b550 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=c223dd45b550
Results (out of 221 total builds):
    exception: 5
    success: 141
    warnings: 34
    failure: 41
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-c223dd45b550

Comment 10

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f7b3bfdcd4e4
https://hg.mozilla.org/integration/mozilla-inbound/rev/45bf5d610be2

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f7b3bfdcd4e4
https://hg.mozilla.org/mozilla-central/rev/45bf5d610be2

Comment 12

[(no longer active)]

Adding a dependency to make sure that bug 612128 does not break this.