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

[Michael Sheldon]

Restore a user's original input if they press backspace immediately after auto-completing a word.

Restore a user's original input if they press backspace immediately after auto-completing a word.

[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:287
http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-ci/534/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-vivid-amd64-ci/35
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-vivid-armhf-ci/35
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-vivid-armhf-ci/35/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-vivid-i386-ci/35

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

review: Approve (continuous-integration)

[Bill Filler (bfiller)]

Found one issue. If you use the cursor to re-position to a different word after autocompleting a word and then backspace the previous is word is replaced incorrectly.

For example type:
"I beleive" (space to autocomplete believe) "you" (space to complete)
then move cursor after "believe" and press backspace and "believe" is replaced with "you"

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:289
http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-ci/543/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-vivid-amd64-ci/44
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-vivid-armhf-ci/44
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-vivid-armhf-ci/44/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-vivid-i386-ci/44/console

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

review: Needs Fixing (continuous-integration)

[Bill Filler (bfiller)]

Works well in qml, but doesn't work in Gmail. Try to compose a mail and pressing backspace after auto-completing does not revert the word.

review: Needs Fixing

[Bill Filler (bfiller)]

Strange it works well for a while and then stops working. Seems to happen after some newlines and then completing and reverting words. When it gets into the broken state it stops working from then on.

[Bill Filler (bfiller)]

going to approve this now as it appears to be an oxide bug

review: Approve

=== modified file 'src/lib/logic/wordengine.cpp'
--- src/lib/logic/wordengine.cpp	2014-12-08 14:26:19 +0000
+++ src/lib/logic/wordengine.cpp	2015-02-02 15:06:53 +0000
@@ -346,6 +346,14 @@
         primary.setPrimary(true);
         d->candidates->replace(0, primary);
         Q_EMIT primaryCandidateChanged(primary.word());
+    } else if (d->currentText && d->currentText->restoredPreedit()) {
+        // The pre-edit has just been restored by the user pressing backspace after
+        // auto-completing a word, so the user input should be the primary candidate
+        WordCandidate primary = d->candidates->value(0);
+        primary.setPrimary(true);
+        d->candidates->replace(0, primary);
+        Q_EMIT primaryCandidateChanged(primary.word());
+        d->currentText->setRestoredPreedit(false);
     } else if (!d->languagePlugin->languageFeature()->ignoreSimilarity()
                && !similarWords(d->candidates->at(0).word(), d->candidates->at(1).word())) {
         // The prediction is too different to the user input, so the user input 

=== modified file 'src/lib/models/text.cpp'
--- src/lib/models/text.cpp	2013-10-30 14:20:15 +0000
+++ src/lib/models/text.cpp	2015-02-02 15:06:53 +0000
@@ -47,6 +47,7 @@
     , m_surrounding_offset(0)
     , m_face(PreeditDefault)
     , m_cursor_position(0)
+    , m_restored_preedit(false)
 {}
 
 //! Returns current preedit.
@@ -194,4 +195,17 @@
     m_cursor_position = cursor_position;
 }
 
+//! Indicates if the preedit has been restored by the user pressing backspace
+bool Text::restoredPreedit() const
+{
+    return m_restored_preedit;
+}
+
+//! Sets whether the preedit has been recently restored
+//! \param restored Preedit restoration state
+void Text::setRestoredPreedit(bool restored)
+{
+    m_restored_preedit = restored;
+}
+
 }} // namespace Model, MaliitKeyboard

=== modified file 'src/lib/models/text.h'
--- src/lib/models/text.h	2013-10-30 17:12:49 +0000
+++ src/lib/models/text.h	2015-02-02 15:06:53 +0000
@@ -55,6 +55,7 @@
     uint m_surrounding_offset; //!< offset of cursor position in surrounding text.
     PreeditFace m_face; //!< face of preedit.
     int m_cursor_position; //!< position of cursor in preedit string.
+    bool m_restored_preedit; //!< indicates that the preedit has just been restored by the user pressing backspace
 
 public:
     explicit Text();
@@ -82,6 +83,9 @@
 
     int cursorPosition() const;
     void setCursorPosition(int cursor_position);
+
+    bool restoredPreedit() const;
+    void setRestoredPreedit(bool restored);
 };
 
 }} // namespace Model, MaliitKeyboard

=== modified file 'src/view/abstracttexteditor.cpp'
--- src/view/abstracttexteditor.cpp	2015-01-08 09:13:21 +0000
+++ src/view/abstracttexteditor.cpp	2015-02-02 15:06:53 +0000
@@ -294,6 +294,8 @@
     int backspace_word_acceleration;
     int deleted_words;
     QString keyboardState;
+    QString previous_preedit;
+    int previous_preedit_position;
 
     explicit AbstractTextEditorPrivate(const EditorOptions &new_options,
                                        Model::Text *new_text,
@@ -323,6 +325,8 @@
     , backspace_word_acceleration(0)
     , deleted_words(0)
     , keyboardState("CHARACTERS")
+    , previous_preedit("")
+    , previous_preedit_position(0)
 {
     auto_repeat_backspace_timer.setSingleShot(true);
     (void) valid();
@@ -460,6 +464,8 @@
         const bool replace_preedit = d->auto_correct_enabled && not d->text->primaryCandidate().isEmpty() && 
                     not d->text->preedit().isEmpty() && isSeparator;
 
+        d->previous_preedit = "";
+
         if (d->preedit_enabled) {
             if (d->text->surroundingRight().left(1).contains(QRegExp("[\\w]")) || email_detected) {
                 // We're editing in the middle of a word or entering an email address, so just insert characters directly
@@ -556,6 +562,8 @@
             } else {
                 space = d->appendix_for_previous_preedit = d->word_engine->languageFeature()->appendixForReplacedPreedit(d->text->preedit());
             }
+            d->previous_preedit = d->text->preedit();
+            d->previous_preedit_position = d->text->surroundingOffset();
             d->text->setPreedit(d->text->primaryCandidate());
         }
         else if (look_for_a_double_space && not stopSequence.isEmpty()) {
@@ -1118,6 +1126,15 @@
                 singleBackspace();
             }
 
+            if (!d->previous_preedit.isEmpty()) {
+                int deletePos = d->text->surroundingOffset() - d->previous_preedit_position - recreatedPreedit.size();
+                if (deletePos == 0 || deletePos == 1) {
+                    recreatedPreedit = d->previous_preedit;
+                    text()->setRestoredPreedit(true);
+                }
+                d->previous_preedit = "";
+            }
+
             replaceTextWithPreedit(recreatedPreedit, 0, 0, recreatedPreedit.size());
         }
 

=== modified file 'tests/autopilot/ubuntu_keyboard/tests/test_keyboard.py'
--- tests/autopilot/ubuntu_keyboard/tests/test_keyboard.py	2015-01-08 18:07:27 +0000
+++ tests/autopilot/ubuntu_keyboard/tests/test_keyboard.py	2015-02-02 15:06:53 +0000
@@ -527,6 +527,33 @@
             Eventually(Equals(expected))
         )
 
+    def test_restore_preedit(self):
+        """Pressing delete after autocompleting a word should restore
+           the original preedit state.
+
+        """
+        text_area = self.launch_test_input_area()
+        self.ensure_focus_on_input(text_area)
+        keyboard = Keyboard()
+        self.addCleanup(keyboard.dismiss)
+
+        keyboard.type('Helfn ')
+
+        expected = "Helen "
+        self.assertThat(
+            text_area.text,
+            Eventually(Equals(expected))
+        )
+
+        keyboard.type('\b ')
+
+        expected = "Helfn "
+        self.assertThat(
+            text_area.text,
+            Eventually(Equals(expected))
+        )
+        
+
     def test_long_press(self):
         """Long pressing a key should enter the default extended character.