Greetings, Unfortunately, Editor::respondToChangedSelection() has platform-specific code that is hard to understand it as listed below. We should move this code to the WebKit layer for better readability. #if (!PLATFORM(MAC) && !PLATFORM(CHROMIUM)) || (PLATFORM(MAC) && (defined(BUILDING_ON_LEOPARD) || defined(BUILDING_ON_SNOW_LEOPARD))) #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 // This only erases markers that are in the first unit (word or sentence) of the selection. // Perhaps peculiar, but it matches AppKit on these Mac OS X versions. if (RefPtr<Range> wordRange = newAdjacentWords.toNormalizedRange()) m_frame->document()->markers()->removeMarkers(wordRange.get(), DocumentMarker::Spelling); #endif #endif Regards, Hironori Bono
Created attachment 142188 [details] The proposed solution 1 Greetings, I have written a change that adds a virtual function TextCheckerClient::shouldEraseMarkersAfterChangeSelection to remove these #if,..,#endif blocks. I would like to set r? to try this change on EWS bots to check if they can compile this change. (I have verified I can compile this change on Chromium, Mac, Win, and Qt, though.) Regards, Hironori Bono
Created attachment 142413 [details] The proposed solution 2 (copied a comment) Greetings Niwa-san, Thanks for your review. I would like to update this change to add a simplified version of the comment in Editor::respondToChangedSelection() to WebKit/mac and WebKit2. Is it OK to land this change when EWS bots compile it? Regards, Hironori Bono
Comment on attachment 142413 [details] The proposed solution 2 (copied a comment) View in context: https://bugs.webkit.org/attachment.cgi?id=142413&action=review > Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm:704 > + // This prevents erasing spelling markers on OS X Lion or later to match AppKit on these Mac OS X versions. > +#if !defined(BUILDING_ON_LEOPARD) && !defined(BUILDING_ON_SNOW_LEOPARD) > + return type != TextCheckingTypeSpelling; > +#else > + return true; > +#endif On my second thought, it's not great that this code is duplicated in WebKit and WebKit2. Is there any way to share this code?
+ap, +darin, +enrica for they might know ways to share code between WebKit/mac and WebKit2/mac.
I don't think that there is a great way to do this. Generally, pushing down to WebCore works best, however is seems appropriate to have policy decisions like this implemented in WebKit.
Comment on attachment 142413 [details] The proposed solution 2 (copied a comment) (In reply to comment #5) > I don't think that there is a great way to do this. Generally, pushing down to WebCore works best, however is seems appropriate to have policy decisions like this implemented in WebKit. Okay. I guess we'll have to duplicate the code then.
Comment on attachment 142413 [details] The proposed solution 2 (copied a comment) Clearing flags on attachment: 142413 Committed r117470: <http://trac.webkit.org/changeset/117470>
All reviewed patches have been landed. Closing bug.