Summary: | [Refactoring] Move platform-specific code in Editor::respondToChangedSelection to the WebKit layer | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hironori Bono <hbono> | ||||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ap, darin, enrica, japhet, jiapu.mail, morrita, rakuco, rniwa, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Hironori Bono
2012-05-16 01:00:22 PDT
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. |