Summary: | Spellcheck should be able to run asynchronously | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hajime Morrita <morrita> | ||||||||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | dglazkov, hbono, mifenton, ossy, rniwa, shinyak, shinyak, tkent, tonikitoo, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 65849, 72849, 72942, 56368, 72655, 72847, 72939, 72940, 73003 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Hajime Morrita
2011-11-09 19:24:07 PST
Created attachment 115831 [details]
Patch
Comment on attachment 115831 [details] Patch Attachment 115831 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10369720 New failing tests: editing/pasteboard/paste-table-001.html css2.1/20110323/abspos-containing-block-initial-004b.htm editing/spelling/spellcheck-attribute.html fast/replaced/width100percent-textarea.html editing/spelling/spelling-unified-emulation.html editing/spelling/spelling-attribute-change.html editing/spelling/spelling-hasspellingmarker.html css2.1/20110323/abspos-containing-block-initial-004d.htm editing/spelling/spelling-linebreak.html editing/spelling/spelling-attribute-at-child.html editing/spelling/spelling.html Created attachment 116164 [details]
Patch
Comment on attachment 116164 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=116164&action=review Is it possible to add a test that modify the checking range content during the spellcheck request is in flight? > Source/WebCore/ChangeLog:7 > + Could you add some detailed explanation here? The bug summary isn't enough. > Source/WebCore/editing/Editor.cpp:1882 > + if (asynchronous) It looks async check and m_spellChecker invocation can be a part of markAllMisspellingsAndBadGrammarInRanges(). > Source/WebCore/editing/SpellChecker.cpp:134 > + if (shouldMarkGrammar) Maybe body of this if() statement should be a part of the if clause above. Created attachment 116202 [details]
Patch
Created attachment 116205 [details]
Patch
Comment on attachment 116205 [details] Patch Clearing flags on attachment: 116205 Committed r101002: <http://trac.webkit.org/changeset/101002> All reviewed patches have been landed. Closing bug. (In reply to comment #7) > (From update of attachment 116205 [details]) > Clearing flags on attachment: 116205 > > Committed r101002: <http://trac.webkit.org/changeset/101002> The new editing/spelling/spellcheck-async.html fails on the GTK and on the Qt bot. Have you got any idea why? --- /ramdisk/qt-linux-release/build/layout-test-results/editing/spelling/spellcheck-async-expected.txt +++ /ramdisk/qt-linux-release/build/layout-test-results/editing/spelling/spellcheck-async-actual.txt @@ -5,14 +5,14 @@ PASS successfullyParsed is true TEST COMPLETE -PASS text : "zz" has markers: [zz] -PASS text : "apple,zz,orange" has markers: [zz] -PASS text : "zz,zz" has markers: [zz,zz] -PASS text : "zz zz zz" has markers: [zz,zz,zz] -PASS text : " zz zz zz " has markers: [zz,zz,zz] -PASS text : "zz apple orange" has markers: [zz] -PASS text : "apple zz orange" has markers: [zz] -PASS text : "apple orange zz" has markers: [zz] +FAIL text : "zz" should have markers: [zz] +FAIL text : "apple,zz,orange" should have markers: [zz] +FAIL text : "zz,zz" should have markers: [zz,zz] +FAIL text : "zz zz zz" should have markers: [zz,zz,zz] +FAIL text : " zz zz zz " should have markers: [zz,zz,zz] +FAIL text : "zz apple orange" should have markers: [zz] +FAIL text : "apple zz orange" should have markers: [zz] +FAIL text : "apple orange zz" should have markers: [zz] PASS text : "zzz" has markers: [] PASS text : "I would like to sleep, zzz" has markers: [] --- /var/lib/buildbot/build/gtk-linux-32-release/build/layout-test-results/editing/spelling/spellcheck-async-expected.txt +++ /var/lib/buildbot/build/gtk-linux-32-release/build/layout-test-results/editing/spelling/spellcheck-async-actual.txt @@ -13,6 +13,6 @@ PASS text : "zz apple orange" has markers: [zz] PASS text : "apple zz orange" has markers: [zz] PASS text : "apple orange zz" has markers: [zz] -PASS text : "zzz" has markers: [] -PASS text : "I would like to sleep, zzz" has markers: [] +FAIL text : "zzz" should have markers: [] +FAIL text : "I would like to sleep, zzz" should have markers: [] Hmm... Though I have to investigate why to say the correct things..., perhaps Qt and GTK have not implemented requestCheckingFor. Or, some functions might not be implemented... Since my previous patch which resolves issues depending on this issue have also failed in Qt and GTK (their tests are skipped on Qt and Gtk now.), resolving them may fix such errors. And sorry, I should have to add this test on Skipped lists. (In reply to comment #9) > (In reply to comment #7) > > (From update of attachment 116205 [details] [details]) > > Clearing flags on attachment: 116205 > > > > Committed r101002: <http://trac.webkit.org/changeset/101002> > > The new editing/spelling/spellcheck-async.html fails on the GTK and on the Qt bot. Have you got any idea why? > > --- /ramdisk/qt-linux-release/build/layout-test-results/editing/spelling/spellcheck-async-expected.txt > +++ /ramdisk/qt-linux-release/build/layout-test-results/editing/spelling/spellcheck-async-actual.txt > @@ -5,14 +5,14 @@ > PASS successfullyParsed is true > > TEST COMPLETE > -PASS text : "zz" has markers: [zz] > -PASS text : "apple,zz,orange" has markers: [zz] > -PASS text : "zz,zz" has markers: [zz,zz] > -PASS text : "zz zz zz" has markers: [zz,zz,zz] > -PASS text : " zz zz zz " has markers: [zz,zz,zz] > -PASS text : "zz apple orange" has markers: [zz] > -PASS text : "apple zz orange" has markers: [zz] > -PASS text : "apple orange zz" has markers: [zz] > +FAIL text : "zz" should have markers: [zz] > +FAIL text : "apple,zz,orange" should have markers: [zz] > +FAIL text : "zz,zz" should have markers: [zz,zz] > +FAIL text : "zz zz zz" should have markers: [zz,zz,zz] > +FAIL text : " zz zz zz " should have markers: [zz,zz,zz] > +FAIL text : "zz apple orange" should have markers: [zz] > +FAIL text : "apple zz orange" should have markers: [zz] > +FAIL text : "apple orange zz" should have markers: [zz] > PASS text : "zzz" has markers: [] > PASS text : "I would like to sleep, zzz" has markers: [] > > --- /var/lib/buildbot/build/gtk-linux-32-release/build/layout-test-results/editing/spelling/spellcheck-async-expected.txt > +++ /var/lib/buildbot/build/gtk-linux-32-release/build/layout-test-results/editing/spelling/spellcheck-async-actual.txt > @@ -13,6 +13,6 @@ > PASS text : "zz apple orange" has markers: [zz] > PASS text : "apple zz orange" has markers: [zz] > PASS text : "apple orange zz" has markers: [zz] > -PASS text : "zzz" has markers: [] > -PASS text : "I would like to sleep, zzz" has markers: [] > +FAIL text : "zzz" should have markers: [] > +FAIL text : "I would like to sleep, zzz" should have markers: [] It seems like the test added by this patch has been failing on Mac: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=editing%2Fspelling%2Fspellcheck-async.html&group=%40ToT%20-%20webkit.org (In reply to comment #11) > It seems like the test added by this patch has been failing on Mac: > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=editing%2Fspelling%2Fspellcheck-async.html&group=%40ToT%20-%20webkit.org I'll check it soon. I've confirmed the test fails occasionally, sometimes the test passes though. I've filed the bug of this flakiness. I'll try to fix soon. https://bugs.webkit.org/show_bug.cgi?id=74388 |