Bug 32905 - With Pinyin Simplified IM, a wrong character is deleted from google.com suggestion
: With Pinyin Simplified IM, a wrong character is deleted from google.com sugge...
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: HTML Editing
: 528+ (Nightly build)
: Macintosh All
: P2 Normal
Assigned To: Alexey Proskuryakov
http://google.com
: InRadar
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-12-23 10:35 PST by Alexey Proskuryakov
Modified: 2009-12-23 21:50 PST (History)
2 users (show)

See Also:


Attachments
proposed fix (4.00 KB, patch)
2009-12-23 11:24 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2009-12-23 10:35:15 PST
<rdar://problem/7193666>

Steps to reproduce
1. Open www.google.com
2. Choose Pinyin Simplified input method (or ITABC on Leopard)
3. Type h in the search field
4. Press Down key
5. Press delete key

Results: although text caret is at the end of completed text after step 4 (I was getting "hyundai"), the first character ("h") is deleted in step 5.
Comment 1 Alexey Proskuryakov 2009-12-23 11:24:33 PST
Created attachment 45444 [details]
proposed fix
Comment 2 WebKit Review Bot 2009-12-23 11:27:47 PST
style-queue ran check-webkit-style on attachment 45444 [details] without any errors.
Comment 3 Darin Adler 2009-12-23 20:30:19 PST
Comment on attachment 45444 [details]
proposed fix

> +        // An open typing command that disagrees about current selection would cause issues with typing later on.
> +        if (m_lastEditCommand && m_lastEditCommand->isTypingCommand())
> +            static_cast<TypingCommand*>(m_lastEditCommand.get())->closeTyping();

I think this needs to check isOpenForMoreTyping before just calling closeTyping. Luckily there is a helper function for this. You should just write:

    TypingCommand::closeTyping(m_lastEditCommand);

The class member function handles the null check too.

r=me if you change it like I suggest
Comment 4 Alexey Proskuryakov 2009-12-23 20:54:10 PST
> I think this needs to check isOpenForMoreTyping before just calling
> closeTyping.

I wonder why - closeTyping() just sets a boolean member value.

> r=me if you change it like I suggest

I'll make the change, if only for consistency.
Comment 5 Darin Adler 2009-12-23 20:56:55 PST
(In reply to comment #4)
> > I think this needs to check isOpenForMoreTyping before just calling
> > closeTyping.
> 
> I wonder why - closeTyping() just sets a boolean member value.

Guess you're right. Wonder why TypingCommand::closeTyping bothers to check it!
Comment 6 Alexey Proskuryakov 2009-12-23 21:50:11 PST
Committed <http://trac.webkit.org/changeset/52542>.