Merge lp:~michael-sheldon/ubuntu-keyboard/fix-1375951 into lp:ubuntu-keyboard

[Michael Sheldon]

Fix re-entry of preedit when in the middle of a sentence, disable preedit when editing in the middle of a word and re-evaluate autocaps whenever the cursor position changes.

Fix re-entry of preedit when in the middle of a sentence, disable preedit when editing in the middle of a word and re-evaluate autocaps whenever the cursor position changes.

[Michael Sheldon (michael-sheldon)]

Are there any related MPs required for this MP to build/function as expected? Please list.

 * No

Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)

 * Yes

Did you perform an exploratory manual test run of your code change and any related functionality on device or emulator?

 * Yes

Did you successfully run all tests found in your component's Test Plan (https://wiki.ubuntu.com/Process/Merges/TestPlan/ubuntu-keyboard) on device or emulator?

 * Yes

If you changed the UI, was the change specified/approved by design?

 * No change

If you changed UI labels, did you update the pot file?

 * No change

If you changed the packaging (debian), did you add a core-dev as a reviewer to this MP?

 * No change

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:233
http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-ci/475/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-amd64-ci/101
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-armhf-ci/100
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-armhf-ci/100/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-i386-ci/100

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-keyboard-ci/475/rebuild

review: Approve (continuous-integration)

[Bill Filler (bfiller)]

Works quite well except for one issue. If you move the cursor to the end of a word and start backspacing, predit gets correctly enabled and shows word choices. The problem is when you select one (or press the space) bar, an additional space gets inserted after the word, leaving you with a double space.

Example:
"Hello brother and sister."

move cursor to end of brother
start backspacing until only "bro" is left. Then type a "t". Now select "brother" from word ribbon and you are left with:

"Hello brother and sister."

I think the fix would be to only auto insert the space if you detect we are at end of the line if that is possible?

review: Needs Fixing

[lp:~michael-sheldon/ubuntu-keyboard/fix-1375951]

234. By Michael Sheldon on 2014-10-10

Only insert spaces on autocomplete if we're not in the middle of a sentence

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:234
http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-ci/476/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-amd64-ci/102
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-armhf-ci/101
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-armhf-ci/101/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-i386-ci/101

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-keyboard-ci/476/rebuild

review: Approve (continuous-integration)

[lp:~michael-sheldon/ubuntu-keyboard/fix-1375951]

235. By Michael Sheldon on 2014-10-10

Return space behaviour to normal if the user continues typing after editing a word in the middle of a sentence.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:235
http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-ci/477/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-amd64-ci/103
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-armhf-ci/102
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-armhf-ci/102/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-i386-ci/102

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-keyboard-ci/477/rebuild

review: Approve (continuous-integration)

[lp:~michael-sheldon/ubuntu-keyboard/fix-1375951]

236. By Michael Sheldon on 2014-10-10

Don't insert spaces when editing words in the middle of sentences from word ribbon selection

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:236
http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-ci/478/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-amd64-ci/104
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-armhf-ci/103
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-armhf-ci/103/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-i386-ci/103

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-keyboard-ci/478/rebuild

review: Approve (continuous-integration)

[Bill Filler (bfiller)]

tested, all issues fixed

review: Approve

=== modified file 'qml/keys/CharKey.qml'
--- qml/keys/CharKey.qml	2014-09-26 17:41:47 +0000
+++ qml/keys/CharKey.qml	2014-10-10 13:28:23 +0000
@@ -206,8 +206,8 @@
                     extendedKeysSelector.closePopover(); 
                 }
             } else if(!swipedOut) {
-                event_handler.onKeyReleased(valueToSubmit, action);
-
+                // Read this prior to altering autocaps
+                var keyToSend = valueToSubmit; 
                 if (magnifier.currentlyAssignedKey == key) {
                     magnifier.shown = false;
                 }
@@ -222,6 +222,7 @@
                 if (switchBackFromSymbols && panel.state === "SYMBOLS") {
                     panel.state = "CHARACTERS";
                 }
+                event_handler.onKeyReleased(keyToSend, action);
             }
         }
 
 
=== modified file 'src/plugin/inputmethod.cpp'
--- src/plugin/inputmethod.cpp	2014-09-25 13:44:09 +0000
+++ src/plugin/inputmethod.cpp	2014-10-10 13:28:23 +0000
@@ -370,7 +370,7 @@
         d->editor.text()->setSurroundingOffset(position);
 
         updateAutoCaps();
-        checkInitialAutocaps();
+        checkAutocaps();
         d->previous_position = position;
     }
 }
@@ -412,9 +412,9 @@
     updateAutoCaps();
 }
 
-//! \brief InputMethod::checkInitialAutocaps  Checks if the keyboard should be
-//! set to uppercase, because the auto caps is enabled and the text is empty.
-void InputMethod::checkInitialAutocaps()
+//! \brief InputMethod::checkAutocaps  Checks if the keyboard should be
+//! set to uppercase after the cursor position has been changed.
+void InputMethod::checkAutocaps()
 {
     Q_D(InputMethod);
 
@@ -422,12 +422,12 @@
         QString text;
         int position;
         bool ok = d->host->surroundingText(text, position);
-        QString textOnLeft = (d->editor.text()->surroundingLeft() + d->editor.text()->preedit()).trimmed();
-        if (ok && text.isEmpty() && d->editor.text()->preedit().isEmpty() && position == 0) {
+        QString textOnLeft = d->editor.text()->surroundingLeft() + d->editor.text()->preedit();
+        if (ok && ((text.isEmpty() && d->editor.text()->preedit().isEmpty() && position == 0) 
+                || d->editor.wordEngine()->languageFeature()->activateAutoCaps(textOnLeft)
+                || d->editor.wordEngine()->languageFeature()->activateAutoCaps(textOnLeft.trimmed()))) {
             Q_EMIT activateAutocaps();
-        } else if (!d->editor.wordEngine()->languageFeature()->activateAutoCaps(textOnLeft)) {
-            // Clear autocaps if it has been set by us previously being in an
-            // empty field
+        } else {
             Q_EMIT deactivateAutocaps();
         }
     }
 
=== modified file 'src/plugin/inputmethod.h'
--- src/plugin/inputmethod.h	2014-09-10 14:57:52 +0000
+++ src/plugin/inputmethod.h	2014-10-10 13:28:23 +0000
@@ -147,7 +147,7 @@
     Q_SLOT void onLayoutWidthChanged(int width);
     Q_SLOT void onLayoutHeightChanged(int height);
 
-    void checkInitialAutocaps();
+    void checkAutocaps();
 
     const QScopedPointer<InputMethodPrivate> d_ptr;
 };
 
=== modified file 'src/view/abstracttexteditor.cpp'
--- src/view/abstracttexteditor.cpp	2014-09-25 15:18:42 +0000
+++ src/view/abstracttexteditor.cpp	2014-10-10 13:28:23 +0000
@@ -287,6 +287,7 @@
     QString ignore_next_surrounding_text;
     bool look_for_a_double_space;
     bool double_space_full_stop_enabled;
+    bool editing_middle_of_text;
     QString appendix_for_previous_preedit;
     int backspace_word_acceleration;
     QString keyboardState;
@@ -313,6 +314,7 @@
     , ignore_next_surrounding_text()
     , look_for_a_double_space(false)
     , double_space_full_stop_enabled(false)
+    , editing_middle_of_text(false)
     , appendix_for_previous_preedit()
     , backspace_word_acceleration(0)
     , keyboardState("CHARACTERS")
@@ -443,7 +445,12 @@
                     not d->text->preedit().isEmpty() && isSeparator;
 
         if (d->preedit_enabled) {
-            if (replace_preedit) {
+            if (d->text->surroundingRight().left(1).contains(QRegExp("[\\w]"))) {
+                // We're editing in the middle of a word, so just insert characters directly
+                d->text->appendToPreedit(text);
+                commitPreedit();
+                alreadyAppended = true;
+            } else if (replace_preedit) {
                 // this means we should commit the candidate, add the separator and whitespace
                 d->text->setPreedit(d->text->primaryCandidate());
                 d->text->appendToPreedit(text);
@@ -510,6 +517,7 @@
     case Key::ActionSpace: {
         QString space = " ";
         QString textOnLeft = d->text->surroundingLeft() + d->text->preedit();
+        QString textOnRight = d->text->surroundingRight().trimmed();
         bool auto_caps_activated = d->word_engine->languageFeature()->activateAutoCaps(textOnLeft);
         const bool replace_preedit = d->auto_correct_enabled && not d->text->primaryCandidate().isEmpty() && not d->text->preedit().isEmpty();
         const QString stopSequence = d->word_engine->languageFeature()->fullStopSequence();
@@ -520,7 +528,14 @@
         }
 
         if (replace_preedit) {
-            space = d->appendix_for_previous_preedit = d->word_engine->languageFeature()->appendixForReplacedPreedit(d->text->preedit());
+            if (!textOnRight.isEmpty() && d->editing_middle_of_text) {
+                // Don't insert a space if we are correcting a word in the middle of a sentence
+                space = "";
+                d->look_for_a_double_space = false;
+                d->editing_middle_of_text = false;
+            } else {
+                space = d->appendix_for_previous_preedit = d->word_engine->languageFeature()->appendixForReplacedPreedit(d->text->preedit());
+            }
             d->text->setPreedit(d->text->primaryCandidate());
         }
         else if (look_for_a_double_space && not stopSequence.isEmpty()) {
@@ -543,10 +558,6 @@
     case Key::ActionReturn: {
         event_key = Qt::Key_Return;
         keyText = QString("\r");
-
-        if (d->word_engine->languageFeature()->activateAutoCaps(keyText) && d->auto_caps_enabled) {
-            Q_EMIT autoCapsActivated();
-        }
     } break;
 
     case Key::ActionClose:
@@ -672,6 +683,11 @@
     const bool auto_caps_activated = d->word_engine->languageFeature()->activateAutoCaps(d->text->preedit());
     d->appendix_for_previous_preedit = d->word_engine->languageFeature()->appendixForReplacedPreedit(d->text->preedit());
     if (d->auto_correct_enabled) {
+        if (!d->text->surroundingRight().trimmed().isEmpty() && d->editing_middle_of_text) {
+            // Don't insert a space if we are correcting a word in the middle of a sentence
+            d->appendix_for_previous_preedit = "";
+            d->editing_middle_of_text = false;
+        }
         d->text->appendToPreedit(d->appendix_for_previous_preedit);
     }
     commitPreedit();
@@ -943,6 +959,9 @@
         }
     }
 
+    if(!d->text->surroundingRight().trimmed().isEmpty()) {
+        d->editing_middle_of_text = true;
+    }
     d->backspace_sent = true;
 }
 
@@ -1052,8 +1071,8 @@
                 leftWords.removeLast();
                 trimDiff += 1;
             }
-            if(!text()->surroundingRight().trimmed().isEmpty()) {
-                // We don't currently handle reentering preedit in the middle of the text
+            if(d->text->surroundingRight().left(1).contains(QRegExp("[\\w]"))) {
+                // Don't enter pre-edit in the middle of a word
                 return;
             }
             QString recreatedPreedit = leftWords.last();
@@ -1062,8 +1081,6 @@
                 // as the last backspace hasn't been committed yet.
                 recreatedPreedit.chop(1);
             }
-            int position = currentOffset - recreatedPreedit.size();
-            QString surroundWithoutPreedit = text()->surrounding().remove(position, recreatedPreedit.size());
 
             for(int i = 0; i < recreatedPreedit.size(); i++) {
                 singleBackspace();