Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undo commit is not working in ibus-mozc #227

Closed
GoogleCodeExporter opened this issue Apr 22, 2015 · 1 comment
Closed

Undo commit is not working in ibus-mozc #227

GoogleCodeExporter opened this issue Apr 22, 2015 · 1 comment

Comments

@GoogleCodeExporter
Copy link

@GoogleCodeExporter GoogleCodeExporter commented Apr 22, 2015

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
Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Apr 22, 2015

This issue was closed by revision r234.

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

  • Changed state: Fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.