google-code-export / mozc

Automatically exported from code.google.com/p/mozc
0 stars 0 forks source link

Undo commit is not working in ibus-mozc #227

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Module: ibus-mozc
Version: Mozc 1.15.1785.102 (r192) and later
OS: Reproduced on Ubuntu 12.04. Probably reproducible on other distros.

What steps will reproduce the problem?
1. Make sure the key-bindings setting of Mozc is MS-IME
2. Open gedit
3. Turn on Mozc
4. Type "ねこだいすき" then hit space key to convert it to "猫大好き" 
then hit enter key to commit it.
5. Hit Ctrl+BS to undo the previous commit.

What is the expected output?
After step 5, the previous commit is canceled and you can continue converting 
"猫大好き".

What do you see instead?
Nothing happens.  Ctrl+BS is not consumed by Mozc and simply passed to the 
application.

Please use labels and text to provide additional information.
This is a recent regression unexpectedly introduced in 1.13.1651.102 (r185), 
which aimed to address Issue 201.

Here is the culprit.
https://code.google.com/p/mozc/source/browse/trunk/src/unix/ibus/mozc_engine.cc?
r=185
> 240: MozcEngine::MozcEngine()
> 241:     : last_sync_time_(Util::GetTime()),
> 242:       key_event_handler_(new KeyEventHandler),
> 243:       client_(client::ClientFactory::NewClient()),
> 244: #ifdef MOZC_ENABLE_X11_SELECTION_MONITOR
> 245:       selection_monitor_(SelectionMonitorFactory::Create(1024)),
> 246: #endif  // MOZC_ENABLE_X11_SELECTION_MONITOR
> 247:       property_handler_(new PropertyHandler(
> 248:           new LocaleBasedMessageTranslator(GetMessageLocale()), 
client_.get())),
> 249:       preedit_handler_(new PreeditHandler()),
> 250: #ifdef ENABLE_GTK_RENDERER
> 251:       gtk_candidate_window_handler_(new GtkCandidateWindowHandler(
> 252:           new renderer::RendererClient())),
> 253: #else
> 254:       gtk_candidate_window_handler_(NULL),
> 255: #endif  // ENABLE_GTK_RENDERER
> 256:       ibus_candidate_window_handler_(new IBusCandidateWindowHandler()),
> 257:       preedit_method_(config::Config::ROMAN) {
> 258:   // Currently client capability is fixed.
> 259:   commands::Capability capability;
> 260:   
capability.set_text_deletion(commands::Capability::DELETE_PRECEDING_TEXT);
> 261:   client_->set_client_capability(capability);
> 262:
> 263: #ifdef MOZC_ENABLE_X11_SELECTION_MONITOR
> 264:   if (selection_monitor_.get() != NULL) {
> 265:     selection_monitor_->StartMonitoring();
> 266:   }
> 267: #endif  // MOZC_ENABLE_X11_SELECTION_MONITOR
> 268:
> 269:   // TODO(yusukes): write a unit test to check if the capability is set
> 270:   // as expected.
> 271: }

The above code had worked perfectly until 1.15.1785.102 (r192), where slightly 
modified the constructor of PropertyHandler to send the initial state to the 
mozc_server.

This is what is going on now:
1. At line 243, |client_| is initialized with a new client.
2. At line 247, the constructor of PropertyHandler internally calls 
ClientInterface::SendCommand, which internally creates a new session to the 
converter.
3. At line 259-261, we update the client capability to indicate that the client 
is able to delete preceding text. But this capability will never be passed to 
the converter because the session has already been created at step 2.

As you can see, ClientInterface::SendCommand is called before the client 
capability is configured.
https://code.google.com/p/mozc/source/diff?spec=svn192&r=192&format=side&path=/t
runk/src/unix/ibus/property_handler.cc&old_path=/trunk/src/unix/ibus/property_ha
ndler.cc&old=185

Calling ClientInterface::set_client_capability just after the client is created 
should resolve the issue.

Original issue reported on code.google.com by yukawa@google.com on 22 Jun 2014 at 3:54

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r234.

Original comment by yukawa@google.com on 22 Jun 2014 at 4:35