In order to avoid using PLATFORM guards to have different editing behaviors on iOS we should have an editing behavior that is specific to iOS.
Created attachment 238228 [details] Patch
Attachment 238228 [details] did not pass style-queue: ERROR: Source/WebCore/editing/DeleteSelectionCommand.cpp:868: Consider using toText helper function in WebCore/dom/Text.h instead of static_cast<Text*> [readability/check] [4] Total errors found: 1 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 238228 [details] Patch Looks good! Now that we have this as a behavior, can we add a test for it.
Comment on attachment 238228 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238228&action=review > Source/WebCore/editing/FrameSelection.cpp:619 > - if (m_frame && m_frame->settings().editingBehaviorType() == EditingMacBehavior) > + if (m_frame && (m_frame->settings().editingBehaviorType() == EditingMacBehavior || m_frame->settings().editingBehaviorType() == EditingIOSBehavior)) We should extract this into a helper function in EditingBehavior.h, not that you should do it in this patch. > Source/WebCore/testing/InternalSettings.cpp:353 > + else if (equalIgnoringCase(editingBehavior, "ios")) > + settings()->setEditingBehaviorType(EditingIOSBehavior); Are you gonna update existing tests in a follow up patch?
> We should extract this into a helper function in EditingBehavior.h, not that you should do it in this patch. > Agreed. This is the only place where we check the EditingBehaviorType directly. > > Source/WebCore/testing/InternalSettings.cpp:353 > > + else if (equalIgnoringCase(editingBehavior, "ios")) > > + settings()->setEditingBehaviorType(EditingIOSBehavior); > > Are you gonna update existing tests in a follow up patch? All the tests currently set the editing behavior as Mac and they should all continue to work. We don't have any test that currently test the iOS behavior, but this change will allow us to have some.
(In reply to comment #5) > > We should extract this into a helper function in EditingBehavior.h, not that you should do it in this patch. > > > Agreed. This is the only place where we check the EditingBehaviorType directly. > > > > Source/WebCore/testing/InternalSettings.cpp:353 > > > + else if (equalIgnoringCase(editingBehavior, "ios")) > > > + settings()->setEditingBehaviorType(EditingIOSBehavior); > > > > Are you gonna update existing tests in a follow up patch? > All the tests currently set the editing behavior as Mac and they should all continue to work. > We don't have any test that currently test the iOS behavior, but this change will allow us to have some. Hm, I probably should have had you break this up into two patches then, one to add the ability to test the iOS behavior, and one that actually changed the code and added a test. Please do add a test though.