Bug 85840 – REGRESSION (r101575): Chinese input is broken when composing mail in iCloud using Safari

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Enrica Casucci]

* STEPS TO REPRODUCE
1. login to iCloud using Safari
2. compose a new mail
3. choose Pinyin - Simplified as input source
4. type "jintian" in the mail body

* RESULTS
After step4, "jin ti" is committed directly and the candidate window shows the candidates for "an".

<rdar://problem/11115520>

[Enrica Casucci]

Created attachment 140615 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 140615 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=140615&action=review

Enrica and myself looked into making a regression test, and determined it impractical. We don't want adding new tests for DOM modification events, and simulating what the page does without mutation events cannot be tested because we don't have appropriate input context hooks.

> Source/WebCore/editing/InsertTextCommand.cpp:119
> +        deleteSelection(false, true, true, false, false);

We should make this readable by using enums or a parameter struct.

[Enrica Casucci]

(In reply to comment #2)
> (From update of attachment 140615 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=140615&action=review
> 
> Enrica and myself looked into making a regression test, and determined it impractical. We don't want adding new tests for DOM modification events, and simulating what the page does without mutation events cannot be tested because we don't have appropriate input context hooks.
> 
> > Source/WebCore/editing/InsertTextCommand.cpp:119
> > +        deleteSelection(false, true, true, false, false);
> 
> We should make this readable by using enums or a parameter struct.

I agree. It was already very unreadable. I want to get rid of all these parameters and replace them with an enum whose values can be combined with an or. I will do that in a separate patch.
Thanks for the review.

[Enrica Casucci]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 140615 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=140615&action=review
> > 
> > Enrica and myself looked into making a regression test, and determined it impractical. We don't want adding new tests for DOM modification events, and simulating what the page does without mutation events cannot be tested because we don't have appropriate input context hooks.
> > 
> > > Source/WebCore/editing/InsertTextCommand.cpp:119
> > > +        deleteSelection(false, true, true, false, false);
> > 
> > We should make this readable by using enums or a parameter struct.
> 
> I agree. It was already very unreadable. I want to get rid of all these parameters and replace them with an enum whose values can be combined with an or. I will do that in a separate patch.
> Thanks for the review.

I want to cleanup deleteSelection in CompositeEditCommand and DeleteSelectionCommand, to avoid keeping separate member variables.

[Ryosuke Niwa]

Closing the bug since the patch appears to have been landed in http://trac.webkit.org/changeset/116368.