Bug 32905

Summary: With Pinyin Simplified IM, a wrong character is deleted from google.com suggestion
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: HTML EditingAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: All   
URL: http://google.com
Attachments:
Description Flags
proposed fix darin: review+

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>.