See testcase, when right-clicking on the text in the input or the textarea, you don't get spelling suggestions for the misspelled word. Also, the "Check Spelling" option is checked by default. It seems to work for the contenteditable node only. As a sidenote (this should probably be a new bug), the text input and the textarea can get a selected look, when you select it. This should not happen. It doesn't happen anymore when you've focused the text input or the textarea and select afterwards.

Peter, is this patch still good? Any plans to get it in the tree?

Looks like that patch has been forgotten. I have send it to the tryserver to check if it still applies and fixes this particular issue.

Not so much forgotten, I just haven't had the time to check whether it is the right fix. (Without an automated testcase sending it to try isn't really useful either)

Requesting blocking as this bug happens on facebook in the status update input field. I believe they switched to a contenteditable field in the last couple of months. We confirmed that this isn't a regression as it appears in the build the patch for bug 237964 was included in the 2007-06-28 nightly builds. Should we add top50 as a keyword since facebook.com is #1 ranked sitein many countries according to alexa.com? I'm not sure if that is reserved for only specific site bugs or not.

(In reply to comment #5) > Not so much forgotten, I just haven't had the time to check whether it is the > right fix. (Without an automated testcase sending it to try isn't really useful > either) Why it's not really useful either? We can at least test manually and check it against existing testcases like we have on bug 484181. Try server builds are available here: http://build.mozilla.org/tryserver-builds/hskupin@mozilla.com-bug433860-spellchecker Running a quick check with attachment 411174 [details] on Windows shows me that we still suffer from the problem even with the proposed patch applied. Given that I would assume that it's not the right fix.

Maybe somebody could write an automated testcase?

(In reply to comment #8) > Maybe somebody could write an automated testcase? Cristian Klein was working recently on a fix and test for bug 370436. CC'ing him and ask if he has an interest for this bug.

(In reply to comment #9) > (In reply to comment #8) > > Maybe somebody could write an automated testcase? > > Cristian Klein was working recently on a fix and test for bug 370436. CC'ing > him and ask if he has an interest for this bug. Hello, At first glance, the bug seems more difficult than the one I fixed. I won't have time until the end of next week to look more into it. OTOH, I found the following comment in extensions/spellcheck/src/mozInlineSpellWordUtil.cpp @ 113: // Find the root node for the editor. For contenteditable we'll need something // cleverer here. Might this be the key to solving this bug?

Peterv's approach was correct. I basically took his patch and added a unit test for input[spellcheck="true"], textareas and contenteditables. I had to tweak the test_contextmenu.html checks a bit to make things work for testing spell check related items in the menu.

I wish there were a way for bugzilla to check whether I've qrefreshed before attaching a patch. :-)

Comment on attachment 448563 [details] [diff] [review] Patch (v1) - let's rename possibleSpellChecking to onEditableArea - file a followup to remove the canSpellCheck and showItem lines in this contentEditable block (they are redundant - we later end up calling initSpellingItems() which does the same work) - file a followup to make sure that we're resetting all the flags we need to in that block, or to fix things so that we don't have to.

(In reply to comment #14) > (From update of attachment 448563 [details] [diff] [review]) > - let's rename possibleSpellChecking to onEditableArea Done. > - file a followup to remove the canSpellCheck and showItem lines in this > contentEditable block (they are redundant - we later end up calling > initSpellingItems() which does the same work) Bug 569658. > - file a followup to make sure that we're resetting all the flags we need to in > that block, or to fix things so that we don't have to. Bug 569659.

We can haz backport? Does this affect 1.9.1 as well?

Comment on attachment 448563 [details] [diff] [review] Patch (v1) (In reply to comment #17) > We can haz backport? Sure. The test needs bug 569504 though (which I think we should take on branch regardless of this bug). > Does this affect 1.9.1 as well? Yes.

Comment on attachment 448563 [details] [diff] [review] Patch (v1) a=beltzner for mozilla-1.9.1 default and mozilla-1.9.2 default, fixes a problem that affects Facebook due to a change in their code. Needs to land with bug 569504

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7aa0256a2b80 http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d584cd712136 For some reason I thought we're at 1.9.1.15 while checking in. :/

Backed out from 1.9.1 due to mochitest failures: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d1168b311bae s: win32-slave45 1457 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #0 (*chubbiness) name - got "context-undo", expected "*chubbiness" Bug 513558 - mochitest-plain: intermittent "test_contextmenu.html | checking item #10 (context-selectall) enabled state - got false, expected true" Bug 508472 - intermittent failures in test_contextmenu.html | menuitem has an access key 1458 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #0 (*chubbiness) enabled state - got false, expected true 1459 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #1 (spell-add-to-dictionary) name - got "---", expected "spell-add-to-dictionary" 1460 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #1 (spell-add-to-dictionary) enabled state - got null, expected true 1461 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #2 (---) name - got "context-cut", expected "---" 1462 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #3 (context-undo) name - got "context-copy", expected "context-undo" 1464 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #4 (---) name - got "context-paste", expected "---" 1465 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #5 (context-cut) name - got "context-delete", expected "context-cut" 1467 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #6 (context-copy) name - got "---", expected "context-copy" 1468 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #6 (context-copy) enabled state - got null, expected false 1469 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #7 (context-paste) name - got "context-selectall", expected "context-paste" 1470 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #8 (context-delete) name - got "---", expected "context-delete" 1471 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #8 (context-delete) enabled state - got null, expected false 1472 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #9 (---) name - got "spell-check-enabled", expected "---" 1473 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #10 (context-selectall) name - got "spell-dictionaries", expected "context-selectall" 1475 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #11 (---) name - got [], expected "---" 1476 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #12 (spell-check-enabled) name - got undefined, expected "spell-check-enabled" 1477 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #12 (spell-check-enabled) enabled state - got undefined, expected true 1478 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #13 (spell-dictionaries) name - got undefined, expected "spell-dictionaries" 1479 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #13 (spell-dictionaries) enabled state - got undefined, expected true 1492 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking expected number of menu entries - got 24, expected 30 1515 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #0 (spell-no-suggestions) name - got "context-undo", expected "spell-no-suggestions" 1517 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #1 (spell-add-to-dictionary) name - got "---", expected "spell-add-to-dictionary" 1518 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #1 (spell-add-to-dictionary) enabled state - got null, expected true 1519 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #2 (---) name - got "context-cut", expected "---" 1520 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #3 (context-undo) name - got "context-copy", expected "context-undo" 1522 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #4 (---) name - got "context-paste", expected "---" 1523 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #5 (context-cut) name - got "context-delete", expected "context-cut" 1525 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #6 (context-copy) name - got "---", expected "context-copy" 1526 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #6 (context-copy) enabled state - got null, expected false 1527 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #7 (context-paste) name - got "context-selectall", expected "context-paste" 1528 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #8 (context-delete) name - got "---", expected "context-delete" 1529 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #8 (context-delete) enabled state - got null, expected false 1530 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #9 (---) name - got "spell-check-enabled", expected "---" 1531 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #10 (context-selectall) name - got "spell-dictionaries", expected "context-selectall" 1533 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #11 (---) name - got [], expected "---" 1534 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #12 (spell-check-enabled) name - got undefined, expected "spell-check-enabled" 1535 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #12 (spell-check-enabled) enabled state - got undefined, expected true 1536 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #13 (spell-dictionaries) name - got undefined, expected "spell-dictionaries" 1537 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #13 (spell-dictionaries) enabled state - got undefined, expected true 1550 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking expected number of menu entries - got 24, expected 30 1573 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #0 (*prodigality) name - got "context-undo", expected "*prodigality" 1574 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #0 (*prodigality) enabled state - got false, expected true 1575 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #1 (spell-add-to-dictionary) name - got "---", expected "spell-add-to-dictionary" 1576 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #1 (spell-add-to-dictionary) enabled state - got null, expected true 1577 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #2 (---) name - got "context-cut", expected "---" 1578 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #3 (context-undo) name - got "context-copy", expected "context-undo" 1580 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #4 (---) name - got "context-paste", expected "---" 1581 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #5 (context-cut) name - got "context-delete", expected "context-cut" 1583 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #6 (context-copy) name - got "---", expected "context-copy" 1584 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #6 (context-copy) enabled state - got null, expected false 1585 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #7 (context-paste) name - got "context-selectall", expected "context-paste" 1586 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #8 (context-delete) name - got "---", expected "context-delete" 1587 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #8 (context-delete) enabled state - got null, expected false 1588 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #9 (---) name - got "spell-check-enabled", expected "---" 1589 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #10 (context-selectall) name - got "spell-dictionaries", expected "context-selectall" 1591 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #11 (---) name - got [], expected "---" 1592 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #12 (spell-check-enabled) name - got undefined, expected "spell-check-enabled" 1593 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #12 (spell-check-enabled) enabled state - got undefined, expected true 1594 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #13 (spell-dictionaries) name - got undefined, expected "spell-dictionaries" 1595 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking item #13 (spell-dictionaries) enabled state - got undefined, expected true 1608 ERROR TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_contextmenu.html | checking expected number of menu entries - got 24, expected 30

Comment on attachment 448563 [details] [diff] [review] Patch (v1) > var item = menu.ownerDocument.createElement("menuitem"); >- item.setAttribute("label", displayName); >+ item.setAttribute("id", "spell-check-dictionary-" + list[i]); >+ item.label = displayName; The label attribute/property change is wrong since there's no XBL yet. [Ironically you could have used the id property, since that's in IDL.]

That's bug 570321.

On 1.9.1, the test doesn't correctly fire the context menu event, so the popup parent range and offset are not set, therefore the spelling suggestions are not hooked to the context menu. Therefore, testing this on 1.9.1 is not possible in the test suite. I verified that the patch fixes the bug on 1.9.1 manually, and landed it without the test: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9141113ce7c1

verified fixed in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.7pre) Gecko/20100701 Namoroka/3.6.7pre