Bug 164369

Created attachment 293854 [details] First pass

<rdar://problem/29050439>

Comment on attachment 293854 [details] First pass View in context: https://bugs.webkit.org/attachment.cgi?id=293854&action=review > Source/WebCore/editing/Editor.cpp:3250 > + if (oldSelection == newSelection) > + return; We should check this condition at this function is called instead. We already have such a check & an early exit in setSelection. > Source/WebCore/editing/Editor.cpp:3252 > +#if PLATFORM(MAC) Why is this behavior Mac only?

Comment on attachment 293854 [details] First pass View in context: https://bugs.webkit.org/attachment.cgi?id=293854&action=review >> Source/WebCore/editing/Editor.cpp:3250 >> + return; > > We should check this condition at this function is called instead. > We already have such a check & an early exit in setSelection. Sounds good -- I'll move it there. >> Source/WebCore/editing/Editor.cpp:3252 >> +#if PLATFORM(MAC) > > Why is this behavior Mac only? When canceling a composition, we also need to reset the platform's IME state so that the platform doesn't still think that we have a pending composition. On Mac, we accomplish this by discarding the NSTextInputContext's marked text. However, there is no analogous way to accomplish this on iOS. Furthermore, this bug seems to only affect Mac. On iOS, we behave differently when changing selection while there is composition text -- we will commit the composition _without_ changing the focused element, which actually produces the correct behavior of dispatching a `compositionend` on the element containing the composition. For instance, if a user types a composition string into a field on iOS without confirming it and taps to switch input fields, the old input field will remain focused and the composition will be committed. Tapping again (when there is no pending composition) then switches focus. I'm not sure if this is a bug or intended behavior -- if this is a bug, we can actually fix this problem by committing the composition early here, but due to the inability to clear composition state on iOS, doing so will require more investigation.

Comment on attachment 293854 [details] First pass View in context: https://bugs.webkit.org/attachment.cgi?id=293854&action=review >>> Source/WebCore/editing/Editor.cpp:3252 >>> +#if PLATFORM(MAC) >> >> Why is this behavior Mac only? > > When canceling a composition, we also need to reset the platform's IME state so that the platform doesn't still think that we have a pending composition. On Mac, we accomplish this by discarding the NSTextInputContext's marked text. However, there is no analogous way to accomplish this on iOS. > > Furthermore, this bug seems to only affect Mac. On iOS, we behave differently when changing selection while there is composition text -- we will commit the composition _without_ changing the focused element, which actually produces the correct behavior of dispatching a `compositionend` on the element containing the composition. For instance, if a user types a composition string into a field on iOS without confirming it and taps to switch input fields, the old input field will remain focused and the composition will be committed. Tapping again (when there is no pending composition) then switches focus. I'm not sure if this is a bug or intended behavior -- if this is a bug, we can actually fix this problem by committing the composition early here, but due to the inability to clear composition state on iOS, doing so will require more investigation. In that case, we should add an empty implementation in Editor.cpp (wrapped in !PLATFORM(MAC)) and this version in EditorMac.mm.

> In that case, we should add an empty implementation in Editor.cpp (wrapped > in !PLATFORM(MAC)) > and this version in EditorMac.mm. That makes sense. Will do!

Created attachment 293918 [details] Patch

Comment on attachment 293918 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=293918&action=review > Source/WebCore/editing/FrameSelection.cpp:308 > + bool selectionsAreEqual = oldSelection == newSelection; I think it's better to negate the variable and call it didMutateSelection or something.

Created attachment 293953 [details] Patch for landing

The commit-queue encountered the following flaky tests while processing attachment 293953 [details]: inspector/sampling-profiler/call-frame-with-dom-functions.html bug 164399 (author: sbarati@apple.com) The commit-queue is continuing to process your patch.

Comment on attachment 293953 [details] Patch for landing Clearing flags on attachment: 293953 Committed r208406: <http://trac.webkit.org/changeset/208406>