734530 - Facebook comment box disables spell check

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Terry R.]

Log into Facebook. Type a misspelled word into the "What's on your mind?" box, and spell check is functioning.  Enter a misspelled word in a Comment box.  Spell check is disabled.

Someone contacted Facebook, but they were told spell check was a browser feature, and not a Facebook issue.  Here is the link on the closed case.
http://developers.facebook.com/bugs/290435294356411/?browse=search_4f4cdc294c2bb2436078759

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Dupe of bug 707434?

Comment 2

[Terry R.]

(In reply to Ryan VanderMeulen from comment #1)
> Dupe of bug 707434?

Ryan,

Doesn't the filing of this bug "confirm" bug 707434?  This occurs on any machine I've tested it on.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

I was under the impression that disabling spell check was a purposeful decision on Facebook's part. Ehsan?

Comment 4

[Terry R.]

(In reply to Ryan VanderMeulen from comment #3)
> I was under the impression that disabling spell check was a purposeful
> decision on Facebook's part. Ehsan?

Do you have any documentation to support that "decision"?

Comment 5

[Aaron Kelley]

Seeing the same issue here (Firefox 11 on Windows, two different machines).  Also can reproduce in the box to reply to a private message thread on Facebook.

Interesting tidbit:  If I inspect the message box with Firebug, and then focus/unfocus it, spellcheck seems to kick in.

Comment 6

[(no longer active)]

(In reply to Aaron Kelley from comment #5)
> Interesting tidbit:  If I inspect the message box with Firebug, and then
> focus/unfocus it, spellcheck seems to kick in.

OK, I see the problem now.  I'll take a look.

Comment 7

[(no longer active)]

The problem here is that the text box in question has an onfocus handler which causes a reframe which causes nsEditorEventListener::Focus to never be called.

Comment 8

[(no longer active)]

This regression is visible on the highly popular site Facebook.  Nominating to track for Firefox 12 and 13.

Comment 9

[(no longer active)]

Attachment: Patch (v1)

Comment 10

[Mozilla RelEng Bot]

Autoland Patchset:
	Patches: 606085
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=7986be090ed0
Try run started, revision 7986be090ed0. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=7986be090ed0

Comment 11

[Mozilla RelEng Bot]

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

Comment 12

[(no longer active)]

http://tbpl.mozilla.org/?tree=Try&rev=445c5a896588

Comment 13

[Mozilla RelEng Bot]

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

Comment 14

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fb904233c37c

Comment 15

[(no longer active)]

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

[Approval Request Comment]
Regression caused by (bug #): bug 364914
User impact if declined: This bug is visible on facebook.com, which is pretty popular!  This bug makes it so that most of user's comments on Facebook would not be spell checked.
Testing completed (on m-c, etc.): I've tested this against the testcase I have manually constructed, and also against facebook.com.
Risk to taking this patch (and alternatives if risky): The problem is well understood here, I don't believe the patch to be very risky.  The downside of not taking it is that this fix will not reach our users until Firefox 14.  Note that we've broken this in 10, and 11 has already been shipped with this bug. :(
String changes made by this patch: none

Comment 16

[Terry R.]

Given the large amount of Facebook users (who hopefully mostly use Firefox), wouldn't this warrant a patch between versions, to correct this earlier?

Comment 17

[(no longer active)]

(In reply to Terry R. from comment #16)
> Given the large amount of Facebook users (who hopefully mostly use Firefox),
> wouldn't this warrant a patch between versions, to correct this earlier?

Let's first see if we can get Aurora/Beta approval...  :-)

Comment 18

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/fb904233c37c

Comment 19

[Alex Keybl [:akeybl]]

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

[Triage Comment]
Approved for Aurora 13, but we'll discuss uplifting to Beta 12 at today's channel meeting. This isn't a very user-visible regression (a lack of spell checking) and has been around since FF10, so if there's any associated risk my intuition is for this to land first in 13. That being said, I hear Facebook is a popular site or something...

Comment 20

[Alex Keybl [:akeybl]]

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

[Triage Comment]
We discussed this in the channel meeting - we're not confident that the extra testing time on the smaller Aurora population will shake out any regressions. We think that the remaining 5 weeks in the beta cycle will. Approved for Beta 12 as well.

Comment 21

[bogas04]

The problem persists for Messages too... If you type misspell in messages firefox won't underline it

Comment 22

[(no longer active)]

(In reply to bogas04 from comment #21)
> The problem persists for Messages too... If you type misspell in messages
> firefox won't underline it

Ouch, I filed bug 737889 for that.  :/

Comment 23

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-aurora/rev/158500e8fa4e
http://hg.mozilla.org/releases/mozilla-beta/rev/888db2929a6c

Comment 24

[Ioana (away)]

Verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0) Gecko/20100101 Firefox/12.0
20120328051619

Comment 26

[Arun Das]

I like how this is going, though I'm only skimming over the dev-talk. I hope to see this fixed soon, at least by Firefox 12!

Comment 27

[Ryan VanderMeulen [:RyanVM]]

I already told you that it's fixed for 12. But you don't even need to take my word for it. Betas of Firefox 12 are easily available for you to try for yourself.

Comment 28

[Arun Das]

Chill chill! I only just read your comments on the other bug!! Thanks for the info.

Comment 29

[Ioana (away)]

Verified as fixed on:
Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (Windows NT 6.1; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:13.0) Gecko/20100101 Firefox/13.0
BuildID: 20120425123149