Per discussion on the bug 67377, we found that confirmCompositionWithoutDisturbingSelection is only used to cancel composition and the corresponding changeset http://trac.webkit.org/changeset/94497 made a change that enforces this notion. We should rename this function to match the actual behavior now.
Created attachment 106469 [details] cleanup
So what if I really want to confirm? We are depending a lot of this method in our slightly outdated branch as we commit the composition instead of cancelling it in many cases (ie. moving from one field to another etc)
(In reply to comment #2) > So what if I really want to confirm? We are depending a lot of this method in our slightly outdated branch as we commit the composition instead of cancelling it in many cases (ie. moving from one field to another etc) r+ upon kenneth's concern is not an issue. I was about to ask the same: if one want to cancel selection without actually disturbing selection.
(In reply to comment #3) > (In reply to comment #2) > > So what if I really want to confirm? We are depending a lot of this method in our slightly outdated branch as we commit the composition instead of cancelling it in many cases (ie. moving from one field to another etc) > > r+ upon kenneth's concern is not an issue. > > I was about to ask the same: if one want to cancel selection without actually disturbing selection. We kind of need that. That is how our mobile platform works. We commit when loosing focus (could be just moving the cursor position with a touch in the same input field) and before injecting compositions into our virtual keyboard (used when the user touches a word and it becomes a composition)
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > So what if I really want to confirm? We are depending a lot of this method in our slightly outdated branch as we commit the composition instead of cancelling it in many cases (ie. moving from one field to another etc) > > > > r+ upon kenneth's concern is not an issue. > > > > I was about to ask the same: if one want to cancel selection without actually disturbing selection. > > We kind of need that. That is how our mobile platform works. We commit when loosing focus (could be just moving the cursor position with a touch in the same input field) and before injecting compositions into our virtual keyboard (used when the user touches a word and it becomes a composition) Although the patch is not chaning any actual behavior, but cleaning it up instead. So if it is/was broken for us with the patch, it will keep beeing.
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > (In reply to comment #2) > > > > So what if I really want to confirm? We are depending a lot of this method in our slightly outdated branch as we commit the composition instead of cancelling it in many cases (ie. moving from one field to another etc) > > > > > > r+ upon kenneth's concern is not an issue. > > > > > > I was about to ask the same: if one want to cancel selection without actually disturbing selection. > > > > We kind of need that. That is how our mobile platform works. We commit when loosing focus (could be just moving the cursor position with a touch in the same input field) and before injecting compositions into our virtual keyboard (used when the user touches a word and it becomes a composition) > > Although the patch is not chaning any actual behavior, but cleaning it up instead. So if it is/was broken for us with the patch, it will keep beeing. Sure, but the behavior was changed without me being aware of it. Examples of use: http://gitorious.org/+qtwebkit-webkit2-dev/webkit/qtwebkit-webkit2-dev/blobs/master/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp I think we are using it one other place, but I don't have the source handy right now.
(In reply to comment #6) > Sure, but the behavior was changed without me being aware of it. Examples of use: > > http://gitorious.org/+qtwebkit-webkit2-dev/webkit/qtwebkit-webkit2-dev/blobs/master/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp > > I think we are using it one other place, but I don't have the source handy right now. Kenneth and I chatted over this and he's going to investigate further if he can work-around by using other API. I'm going to land this patch for now because I don't want to leave the function name mismatching to its behavior. We can always add new API for Qt if it's necessary.
Comment on attachment 106469 [details] cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=106469&action=review > Source/WebCore/editing/Editor.cpp:1494 > - if (preserveSelection) > + if (mode == ConfirmComposition) Oops, this obviously needs to be "mode == CancelComposition". Will fix before landing the patch.
Committed r94613: <http://trac.webkit.org/changeset/94613>