Created attachment 552033 patch v2.6 > But I just remembered that Android can't handle focus > correctly yet. So, can you just mark the test as needs-focus in the reftest > manifest? That should make Android skip the test.
What do you mean with: can't handle focus correctly ? Because, if I understand correctly, needs-focus just check if content if out-of-process http://hg.mozilla.org/mozilla-central/file/a0e3c589c8fa/layout/tools/reftest/reftest.js#l859 Is this the cause focus fails on Android, or should the directive be skip-if(Android) instead ? I've investigated more about the CheckWords call > >- var curlang = spellchecker.GetCurrentDictionary(); > >+ var curlang = ""; > >+ try { > >+ curlang = spellchecker.GetCurrentDictionary(); > >+ } catch(e) {} > > Also, why is this one needed? this code is executed when selecting a dictionary in context menu. This can happen even when no dictionary is active. So this check is strictly needed. > >- if (! spellchecker.CheckCurrentWord(this.mMisspelling)) > >- return 0; // word seems not misspelled after all (?) > >+ try { > >+ if (! spellchecker.CheckCurrentWord(this.mMisspelling)) > >+ return 0; // word seems not misspelled after all (?) > >+ } catch(e) { > >+ return 0; > >+ } > > Why is this change needed? > I think it's less needed, because this.mOverMisspelling should not be true if there is no misspelled word. But then, spellchecker.CheckCurrentWord is as much unneeded as the try/catch around it. > Then can we just not attempt to do a spell check in that case? Good idea. In attached patch, I return early from mozInlineSpellChecker::ResumeCheck if there is not dictionary available. > >diff -r 61bb2bb510c9 extensions/spellcheck/src/mozInlineSpellChecker.cpp > >+ if (NS_FAILED(rv)) > >+ continue; > > Why is this change needed? Then, it should be less needed. But isn't it better to check for functions results. (This call may return with NS_ERROR_OUT_OF_MEMORY for example) Also, this iteration fixes indentation nit. > > About, when the blur is made inside double timeout, updateCurrentDictionary > > is > > not called. > > Why? I don't known exactly why. But with something like that *in a reftest*, the focus listener is not triggered, event if wrapping the setTimeout inside another setTimeout. var editor = document.getElementById('editor'); editor.addEventListener("focus", function() { // not reached in reftest }, false); window.setTimeout(function() { editor.blur(); }, 0); And we need to trigger the focus listener because that's when UpdateCurrentDictionary is called. Otherwise, test seem to pass, but for the wrong reason: because no dictionary has been set. > Because spellchecking doesn't happen on focus, we post an event to the event > loop and start the spellchecking when that event is processed. So, what I tried to do was using setTimeout inside the focus handler var editor = document.getElementById('editor'); editor.addEventListener("focus", function() { window.setTimeout(function() { editor.addEventListener("blur", function() { document.documentElement.className = ''; }, false); editor.blur(); }, 0); }, false); editor.focus(); Unfortunately, this fails on windows platform http://tinderbox.mozilla.org/showlog.cgi?log=Try/1312952687.1312955723.27999.gz Actually, I already spent more time than initially desired on this issue, and I really don't want to try again to guess what's wrong on a platform I don't even have access to. So, I'll currently stop working and this bug. If someone wants to step in and fix that issue, and the possible other ones remaining, that would be nice; otherwise, I may resume on this later. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/303269 Title: Automatically select language for spell check based on user input To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/303269/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
