Summary: | [Chromium] moving a cursor on a misspelled word should not remove a misspelled underline | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hironori Bono <hbono> | ||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | adele, darin, enrica, jiapu.mail, morrita, rniwa, tkent, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Hironori Bono
2012-04-04 14:19:46 PDT
Created attachment 135691 [details]
A patch v0
Comment on attachment 135691 [details] A patch v0 View in context: https://bugs.webkit.org/attachment.cgi?id=135691&action=review > Source/WebCore/editing/Editor.cpp:2931 > +#if PLATFORM(CHROMIUM) > + if (!m_frame->settings() || !m_frame->settings()->asynchronousSpellCheckingEnabled()) { > + if (RefPtr<Range> wordRange = newAdjacentWords.toNormalizedRange()) > + m_frame->document()->markers()->removeMarkers(wordRange.get(), DocumentMarker::Spelling); > + } > +#else Does this need to be guarded inside Chromium at all? Doesn't other port that implements async spell checking want this fix as well? Comment on attachment 135691 [details] A patch v0 View in context: https://bugs.webkit.org/attachment.cgi?id=135691&action=review >> Source/WebCore/editing/Editor.cpp:2931 >> +#else > > Does this need to be guarded inside Chromium at all? Doesn't other port that implements async spell checking want this fix as well? Unfortunately, it loses compatibility with AppKit on Snow Leopard. Even though AppKit of Snow Leopard can spellcheck text in the background when we paste text, it deletes misspelling markers when we move a cursor. Comment on attachment 135691 [details] A patch v0 View in context: https://bugs.webkit.org/attachment.cgi?id=135691&action=review >>> Source/WebCore/editing/Editor.cpp:2931 >>> +#else >> >> Does this need to be guarded inside Chromium at all? Doesn't other port that implements async spell checking want this fix as well? > > Unfortunately, it loses compatibility with AppKit on Snow Leopard. Even though AppKit of Snow Leopard can spellcheck text in the background when we paste text, it deletes misspelling markers when we move a cursor. Interesting. It seems acceptable to land this patch as is but we should figure out a better way of abstracting these port-specific behaviors. Maybe we want some client callback here? Jpu & morrita: Can you think of a good way to push these port-specific code to WebKit layer? It's really annoying to have all these if-defs in WebCore code. Comment on attachment 135691 [details] A patch v0 Clearing flags on attachment: 135691 Committed r113405: <http://trac.webkit.org/changeset/113405> All reviewed patches have been landed. Closing bug. Greetings Niwa-san, Sorry for my slow response. I have filed Bug 86591 <http://webkit.org/b/86591> to refactor it. Regards, Hironori Bono (In reply to comment #5) > Jpu & morrita: Can you think of a good way to push these port-specific code to WebKit layer? > > It's really annoying to have all these if-defs in WebCore code. (In reply to comment #8) > Sorry for my slow response. I have filed Bug 86591 <http://webkit.org/b/86591> to refactor it. r=me on that patch. Thanks for the follow up! |