Fix IMF manager issues. 20/50120/8
authorVictor Cebollada <v.cebollada@samsung.com>
Thu, 22 Oct 2015 14:52:39 +0000 (15:52 +0100)
committerVictor Cebollada <v.cebollada@samsung.com>
Tue, 27 Oct 2015 08:40:03 +0000 (08:40 +0000)
The text underline while pre-edit text differs from other solutions/toolkits i.e EFL or Android.

One use case that looks different is:
1) Type a word
2) Tap somewhere inside the word
3) Add more text. At this point the underline is visible in Dali but not in EFL or Android.

In addition if the text added in the step 3) is deleted, the underline remains in one character, which is wrong.

Another use case:
1) Type a word
2) Tap at the end of the last word of the text
3) Add more text. At this point the whole last word should be underlined as pre-edit. Sometimes it happens, sometimes it doesn't.

Change-Id: Ib0c76157260776296dfec34680bd200c1b3268f1
Signed-off-by: Victor Cebollada <v.cebollada@samsung.com>
automated-tests/src/dali-toolkit/dali-toolkit-test-utils/toolkit-imf-manager.cpp
automated-tests/src/dali-toolkit/dali-toolkit-test-utils/toolkit-imf-manager.h
dali-toolkit/internal/text/text-controller-impl.cpp
dali-toolkit/internal/text/text-controller-impl.h
dali-toolkit/internal/text/text-controller.cpp

index 0aee8ec..e35ce05 100644 (file)
@@ -21,7 +21,6 @@
 // EXTERNAL INCLUDES
 #include <dali/public-api/object/base-object.h>
 #include <dali/integration-api/debug.h>
-#include <dali/integration-api/events/key-event-integ.h>
 
 namespace Dali
 {
@@ -51,10 +50,10 @@ public:
   bool RestoreAfterFocusLost() const;
   void SetRestoreAfterFocusLost( bool toggle );
   void NotifyCursorPosition();
-  int GetCursorPosition();
   void SetCursorPosition( unsigned int cursorPosition );
-  void SetSurroundingText( std::string text );
-  std::string GetSurroundingText();
+  unsigned int GetCursorPosition() const;
+  void SetSurroundingText( const std::string& text );
+  const std::string& GetSurroundingText() const;
 
 public:  // Signals
   ImfManagerSignalType& ActivatedSignal() { return mActivatedSignal; }
@@ -78,7 +77,6 @@ private:
   bool mRestoreAfterFocusLost:1;             ///< Whether the keyboard needs to be restored (activated ) after focus regained.
   bool mIdleCallbackConnected:1;             ///< Whether the idle callback is already connected.
 
-  std::vector<Dali::Integration::KeyEvent> mKeyEvents; ///< Stores key events to be sent from idle call-back.
   ImfManagerSignalType      mActivatedSignal;
   ImfEventSignalType        mEventSignal;
 
@@ -120,10 +118,9 @@ Dali::ImfManager ImfManager::Get()
 
 ImfManager::ImfManager( /*Ecore_X_Window ecoreXwin*/ )
 : mIMFCursorPosition( 0 ),
-  mSurroundingText(""),
+  mSurroundingText(),
   mRestoreAfterFocusLost( false ),
-  mIdleCallbackConnected( false ),
-  mKeyEvents()
+  mIdleCallbackConnected( false )
 {
   CreateContext( /*ecoreXwin*/ );
   ConnectCallbacks();
@@ -178,22 +175,22 @@ void ImfManager::NotifyCursorPosition()
 {
 }
 
-int ImfManager::GetCursorPosition()
+void ImfManager::SetCursorPosition( unsigned int cursorPosition )
 {
-  return mIMFCursorPosition;
+  mIMFCursorPosition = static_cast< int >( cursorPosition );
 }
 
-void ImfManager::SetCursorPosition( unsigned int cursorPosition )
+unsigned int ImfManager::GetCursorPosition() const
 {
-  mIMFCursorPosition = ( int )cursorPosition;
+  return static_cast<unsigned int>( mIMFCursorPosition );
 }
 
-void ImfManager::SetSurroundingText( std::string text )
+void ImfManager::SetSurroundingText( const std::string& text )
 {
   mSurroundingText = text;
 }
 
-std::string ImfManager::GetSurroundingText()
+const std::string& ImfManager::GetSurroundingText() const
 {
   return mSurroundingText;
 }
@@ -220,11 +217,6 @@ ImfManager ImfManager::Get()
   return Internal::Adaptor::ImfManager::Get();
 }
 
-ImfContext ImfManager::GetContext()
-{
-  return NULL;
-}
-
 void ImfManager::Activate()
 {
   Internal::Adaptor::ImfManager::GetImplementation(*this).Activate();
@@ -260,17 +252,17 @@ void ImfManager::SetCursorPosition( unsigned int SetCursorPosition )
   Internal::Adaptor::ImfManager::GetImplementation(*this).SetCursorPosition( SetCursorPosition );
 }
 
-int ImfManager::GetCursorPosition()
+unsigned int ImfManager::GetCursorPosition() const
 {
   return Internal::Adaptor::ImfManager::GetImplementation(*this).GetCursorPosition();
 }
 
-void ImfManager::SetSurroundingText( std::string text )
+void ImfManager::SetSurroundingText( const std::string& text )
 {
   Internal::Adaptor::ImfManager::GetImplementation(*this).SetSurroundingText( text );
 }
 
-std::string ImfManager::GetSurroundingText()
+const std::string& ImfManager::GetSurroundingText() const
 {
   return Internal::Adaptor::ImfManager::GetImplementation(*this).GetSurroundingText();
 }
index 842078b..d90e9d4 100644 (file)
@@ -34,171 +34,198 @@ class ImfManager;
 }
 }
 
-typedef void* ImfContext;
-
 /**
  * @brief The ImfManager class
+ *
  * Specifically manages the ecore input method framework which enables the virtual or hardware keyboards.
  */
 class ImfManager : public BaseHandle
 {
 public:
 
+  /**
+   * @brief Events that are generated by the IMF.
+   */
   enum ImfEvent
   {
-    VOID,
-    PREEDIT,
-    COMMIT,
-    DELETESURROUNDING,
-    GETSURROUNDING
+    VOID,                ///< No event
+    PREEDIT,             ///< Pre-Edit changed
+    COMMIT,              ///< Commit recieved
+    DELETESURROUNDING,   ///< Event to delete a range of characters from the string
+    GETSURROUNDING       ///< Event to query string and cursor position
   };
 
   /**
-   * This structure is used to pass on data from the IMF regarding predictive text.
+   * @brief This structure is used to pass on data from the IMF regarding predictive text.
    */
   struct ImfEventData
   {
     /**
-     * Default Constructor.
+     * @brief Default Constructor.
      */
     ImfEventData()
-    : eventName( VOID ),
-      predictiveString(""),
+    : predictiveString(),
+      eventName( VOID ),
       cursorOffset( 0 ),
       numberOfChars ( 0 )
     {
     };
 
     /**
-     * Constructor
+     * @brief Constructor
+     *
      * @param[in] aEventName The name of the event from the IMF.
      * @param[in] aPredictiveString The pre-edit or commit string.
      * @param[in] aCursorOffset Start position from the current cursor position to start deleting characters.
      * @param[in] aNumberOfChars The number of characters to delete from the cursorOffset.
      */
-    ImfEventData(ImfEvent aEventName, const std::string& aPredictiveString, int aCursorOffset,int aNumberOfChars  )
-    : eventName(aEventName), predictiveString(aPredictiveString), cursorOffset( aCursorOffset ), numberOfChars( aNumberOfChars )
+    ImfEventData( ImfEvent aEventName, const std::string& aPredictiveString, int aCursorOffset, int aNumberOfChars )
+    : predictiveString( aPredictiveString ),
+      eventName( aEventName ),
+      cursorOffset( aCursorOffset ),
+      numberOfChars( aNumberOfChars )
     {
     }
 
     // Data
-    ImfEvent eventName; // The name of the event from the IMF.
-    std::string predictiveString; // The pre-edit or commit string.
-    int cursorOffset; // Start position from the current cursor position to start deleting characters.
-    int numberOfChars; //number of characters to delete from the cursorOffset.
+    std::string predictiveString; ///< The pre-edit or commit string.
+    ImfEvent eventName;           ///< The name of the event from the IMF.
+    int cursorOffset;             ///< Start position from the current cursor position to start deleting characters.
+    int numberOfChars;            ///< number of characters to delete from the cursorOffset.
   };
 
   /**
-   * Data required my IMF from the callback
+   * @brief Data required by IMF from the callback
    */
   struct ImfCallbackData
   {
-    ImfCallbackData( )
-    : update( false ), cursorPosition( 0 ), preeditResetRequired ( false )
+    /**
+     * @brief Constructor
+     */
+    ImfCallbackData()
+    : currentText(),
+      cursorPosition( 0 ),
+      update( false ),
+      preeditResetRequired( false )
     {
     }
 
-    ImfCallbackData(bool aUpdate, int aCursorPosition, std::string aCurrentText, bool aPreeditResetRequired )
-    : update(aUpdate), cursorPosition(aCursorPosition), currentText( aCurrentText ), preeditResetRequired( aPreeditResetRequired )
+    /**
+     * @brief Constructor
+     * @param[in] aUpdate True if cursor position needs to be updated
+     * @param[in] aCursorPosition new position of cursor
+     * @param[in] aCurrentText current text string
+     * @param[in] aPreeditResetRequired flag if preedit reset is required.
+     */
+    ImfCallbackData( bool aUpdate, int aCursorPosition, const std::string& aCurrentText, bool aPreeditResetRequired )
+    : currentText( aCurrentText ),
+      cursorPosition( aCursorPosition ),
+      update( aUpdate ),
+      preeditResetRequired( aPreeditResetRequired )
     {
     }
 
-    bool update; // if cursor position needs to be updated
-    int cursorPosition; // new position of cursor
-    std::string currentText; // current text string
-    bool preeditResetRequired; // flag if preedit reset is required.
+    std::string currentText;      ///< current text string
+    int cursorPosition;           ///< new position of cursor
+    bool update               :1; ///< if cursor position needs to be updated
+    bool preeditResetRequired :1; ///< flag if preedit reset is required.
   };
 
-  typedef Signal< void (ImfManager&) > ImfManagerSignalType;
-
-  typedef Signal< ImfCallbackData ( ImfManager&, const ImfEventData& ) > ImfEventSignalType;
+  typedef Signal< void (ImfManager&) > ImfManagerSignalType; ///< Keyboard actived signal
+  typedef Signal< ImfCallbackData ( ImfManager&, const ImfEventData& ) > ImfEventSignalType; ///< keyboard events
 
 public:
 
   /**
-   * Retrieve a handle to the instance of ImfManager.
+   * @brief Retrieve a handle to the instance of ImfManager.
    * @return A handle to the ImfManager.
    */
   static ImfManager Get();
 
   /**
-   * Get the current imf context.
-   * @return current imf context.
-   */
-  ImfContext GetContext();
-
-  /**
-   * Activate the IMF.
+   * @brief Activate the IMF.
+   *
    * It means that the text editing is started at somewhere.
    * If the H/W keyboard isn't connected then it will show the virtual keyboard.
    */
   void Activate();
 
   /**
-   * Deactivate the IMF.
+   * @brief Deactivate the IMF.
+   *
    * It means that the text editing is finished at somewhere.
    */
   void Deactivate();
 
   /**
-   * Get the restoration status, which controls if the keyboard is restored after the focus lost then regained.
+   * @brief Get the restoration status, which controls if the keyboard is restored after the focus lost then regained.
+   *
    * If true then keyboard will be restored (activated) after focus is regained.
    * @return restoration status.
    */
   bool RestoreAfterFocusLost() const;
 
   /**
-   * Set status whether the IMF has to restore the keyboard after losing focus.
+   * @brief Set status whether the IMF has to restore the keyboard after losing focus.
+   *
    * @param[in] toggle True means that keyboard should be restored after focus lost and regained.
    */
   void SetRestoreAfterFocusLost( bool toggle );
 
   /**
-   * Send message reset the pred-edit state / imf module.  Used to interupt pre-edit state maybe due to a touch input.
+   * @brief Send message reset the pred-edit state / imf module.
+   *
+   * Used to interupt pre-edit state maybe due to a touch input.
    */
   void Reset();
 
   /**
-   * Notifies IMF context that the cursor position has changed, required for features like auto-capitalisation
+   * @brief Notifies IMF context that the cursor position has changed, required for features like auto-capitalisation.
    */
   void NotifyCursorPosition();
 
   /**
-   * Sets cursor position stored in VirtualKeyboard, this is required by the IMF context
+   * @brief Sets cursor position stored in VirtualKeyboard, this is required by the IMF context.
+   *
    * @param[in] cursorPosition position of cursor
    */
   void SetCursorPosition( unsigned int cursorPosition );
 
   /**
-   * Gets cursor position stored in VirtualKeyboard, this is required by the IMF context
+   * @brief Gets cursor position stored in VirtualKeyboard, this is required by the IMF context.
+   *
    * @return current position of cursor
    */
-  int GetCursorPosition();
+  unsigned int GetCursorPosition() const;
 
   /**
-   * Method to store the string required by the IMF, this is used to provide predictive word suggestions.
+   * @brief Method to store the string required by the IMF, this is used to provide predictive word suggestions.
+   *
    * @param[in] text The text string surrounding the current cursor point.
    */
-  void SetSurroundingText( std::string text );
+  void SetSurroundingText( const std::string& text );
 
   /**
-   * Gets current text string set within the IMF manager, this is used to offer predictive suggestions
+   * @brief Gets current text string set within the IMF manager, this is used to offer predictive suggestions.
+   *
    * @return current position of cursor
    */
-  std::string GetSurroundingText();
+  const std::string& GetSurroundingText() const;
 
 public:
 
   // Signals
 
   /**
-   * This is emitted when the virtual keyboard is connected to or the hardware keyboard is activated.
+   * @brief This is emitted when the virtual keyboard is connected to or the hardware keyboard is activated.
+   *
    * @return The IMF Activated signal.
    */
   ImfManagerSignalType& ActivatedSignal();
 
   /**
-   * This is emitted when the IMF manager receives an event from the IMF
+   * @brief This is emitted when the IMF manager receives an event from the IMF.
+   *
    * @return The Event signal containing the event data.
    */
   ImfEventSignalType& EventReceivedSignal();
@@ -206,20 +233,23 @@ public:
   // Construction & Destruction
 
   /**
-   * Constructor
+   * @brief Constructor.
    */
   ImfManager();
 
   /**
-   * Non virtual destructor.
+   * @brief Destructor
+   *
+   * This is non-virtual since derived Handle types must not contain data or virtual methods.
    */
   ~ImfManager();
 
   /**
-   * This constructor is used by ImfManager::Get().
+   * @brief This constructor is used by ImfManager::Get().
+   *
    * @param[in] imfManager A pointer to the imf Manager.
    */
-  ImfManager( Internal::Adaptor::ImfManager* imfManager );
+  explicit ImfManager( Internal::Adaptor::ImfManager* imfManager );
 };
 
 } // namespace Dali
index b1c0b14..b262e99 100644 (file)
@@ -106,6 +106,7 @@ void GetGlyphsMetrics( GlyphIndex glyphIndex,
 
 EventData::EventData( DecoratorPtr decorator )
 : mDecorator( decorator ),
+  mImfManager(),
   mPlaceholderTextActive(),
   mPlaceholderTextInactive(),
   mPlaceholderTextColor( 0.8f, 0.8f, 0.8f, 0.8f ),
@@ -132,7 +133,9 @@ EventData::EventData( DecoratorPtr decorator )
   mScrollAfterUpdatePosition( false ),
   mScrollAfterDelete( false ),
   mAllTextSelected( false )
-{}
+{
+  mImfManager = ImfManager::Get();
+}
 
 EventData::~EventData()
 {}
@@ -575,6 +578,13 @@ void Controller::Impl::OnTapEvent( const Event& event )
 
       mEventData->mUpdateCursorPosition = true;
       mEventData->mScrollAfterUpdatePosition = true;
+
+      // Notify the cursor position to the imf manager.
+      if( mEventData->mImfManager )
+      {
+        mEventData->mImfManager.SetCursorPosition( mEventData->mPrimaryCursorPosition );
+        mEventData->mImfManager.NotifyCursorPosition();
+      }
     }
   }
 }
index f1c4bc4..2e2c240 100644 (file)
@@ -115,10 +115,11 @@ struct EventData
 
   ~EventData();
 
-  DecoratorPtr       mDecorator;               ///< Pointer to the decorator
-  std::string        mPlaceholderTextActive;   ///< The text to display when the TextField is empty with key-input focus
-  std::string        mPlaceholderTextInactive; ///< The text to display when the TextField is empty and inactive
-  Vector4            mPlaceholderTextColor;    ///< The in/active placeholder text color
+  DecoratorPtr       mDecorator;               ///< Pointer to the decorator.
+  ImfManager         mImfManager;              ///< The Input Method Framework Manager.
+  std::string        mPlaceholderTextActive;   ///< The text to display when the TextField is empty with key-input focus.
+  std::string        mPlaceholderTextInactive; ///< The text to display when the TextField is empty and inactive.
+  Vector4            mPlaceholderTextColor;    ///< The in/active placeholder text color.
 
   /**
    * This is used to delay handling events until after the model has been updated.
@@ -332,10 +333,9 @@ struct Controller::Impl
     if( mEventData )
     {
       // Reset incase we are in a pre-edit state.
-      ImfManager imfManager = ImfManager::Get();
-      if ( imfManager )
+      if( mEventData->mImfManager )
       {
-        imfManager.Reset(); // Will trigger a commit message
+        mEventData->mImfManager.Reset(); // Will trigger a message ( commit, get surrounding )
       }
 
       ClearPreEditFlag();
index e990d42..eddde06 100644 (file)
@@ -453,12 +453,12 @@ bool Controller::RemoveText( int cursorOffset, int numberOfChars )
       cursorIndex = oldCursorIndex + cursorOffset;
     }
 
-    if( (cursorIndex + numberOfChars) > currentText.Count() )
+    if( ( cursorIndex + numberOfChars ) > currentText.Count() )
     {
       numberOfChars = currentText.Count() - cursorIndex;
     }
 
-    if( (cursorIndex + numberOfChars) <= currentText.Count() )
+    if( ( cursorIndex + numberOfChars ) <= currentText.Count() )
     {
       Vector<Character>::Iterator first = currentText.Begin() + cursorIndex;
       Vector<Character>::Iterator last  = first + numberOfChars;
@@ -1520,7 +1520,7 @@ void Controller::TapEvent( unsigned int tapCount, float x, float y )
       else if( EventData::EDITING                  != mImpl->mEventData->mState &&
                EventData::EDITING_WITH_GRAB_HANDLE != mImpl->mEventData->mState )
       {
-        if( mImpl->IsShowingPlaceholderText() &&  ! mImpl->IsFocusedPlaceholderAvailable() )
+        if( mImpl->IsShowingPlaceholderText() &&  !mImpl->IsFocusedPlaceholderAvailable() )
         {
           // Hide placeholder text
           ResetText();
@@ -1806,7 +1806,7 @@ void Controller::TextPopupButtonTouched( Dali::Toolkit::TextSelectionPopup::Butt
 
 ImfManager::ImfCallbackData Controller::OnImfEvent( ImfManager& imfManager, const ImfManager::ImfEventData& imfEvent )
 {
-  bool update( false );
+  bool update = false;
   bool requestRelayout = false;
 
   std::string text;
@@ -1934,17 +1934,15 @@ void Controller::NotifyImfManager()
 {
   if( mImpl->mEventData )
   {
-    ImfManager imfManager = ImfManager::Get();
-
-    if( imfManager )
+    if( mImpl->mEventData->mImfManager )
     {
       // Notifying IMF of a cursor change triggers a surrounding text request so updating it now.
       std::string text;
       GetText( text );
-      imfManager.SetSurroundingText( text );
+      mImpl->mEventData->mImfManager.SetSurroundingText( text );
 
-      imfManager.SetCursorPosition( GetLogicalCursorPosition() );
-      imfManager.NotifyCursorPosition();
+      mImpl->mEventData->mImfManager.SetCursorPosition( GetLogicalCursorPosition() );
+      mImpl->mEventData->mImfManager.NotifyCursorPosition();
     }
   }
 }