The default bug view has changed. See this FAQ.

Spell checking is broken when typing Facebook messages

RESOLVED FIXED in mozilla14

Status

()

Core
Spelling checker
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

({testcase})

unspecified
mozilla14
x86
Mac OS X
testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
This is similar to bug 734530, but is not fixed by the patch there. :(
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.
(Assignee)

Comment 2

5 years ago
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.
Keywords: regression → testcase
(Assignee)

Comment 3

5 years ago
Created attachment 608125 [details] [diff] [review]
Patch (v1)
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #608125 - Flags: review?(roc)
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

5 years ago
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/eakhgari@mozilla.com-063db6b952f4
(Assignee)

Comment 6

5 years ago
Created attachment 608391 [details] [diff] [review]
Patch (v1)

Sorry. that was the wrong patch!
Attachment #608125 - Attachment is obsolete: true
Attachment #608125 - Flags: review?(roc)
Attachment #608391 - Flags: review?(roc)
Comment on attachment 608391 [details] [diff] [review]
Patch (v1)

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

That makes more sense :-)
Attachment #608391 - Flags: review?(roc) → review+
(Assignee)

Comment 8

5 years ago
Created attachment 608516 [details] [diff] [review]
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.
Attachment #608516 - Flags: review?(roc)
Attachment #608516 - Flags: review?(roc) → review+

Comment 9

5 years ago
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/eakhgari@mozilla.com-c223dd45b550
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7b3bfdcd4e4
https://hg.mozilla.org/integration/mozilla-inbound/rev/45bf5d610be2
Flags: in-testsuite+
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/f7b3bfdcd4e4
https://hg.mozilla.org/mozilla-central/rev/45bf5d610be2
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 741216
(Assignee)

Comment 12

5 years ago
Adding a dependency to make sure that bug 612128 does not break this.
Depends on: 612128
You need to log in before you can comment on or make changes to this bug.