Bug 85840 - REGRESSION (r101575): Chinese input is broken when composing mail in iCloud using Safari
Summary: REGRESSION (r101575): Chinese input is broken when composing mail in iCloud u...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Unspecified
: P2 Normal
Assignee: Enrica Casucci
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-05-07 16:04 PDT by Enrica Casucci
Modified: 2012-05-10 01:17 PDT (History)
1 user (show)

See Also:


Attachments
Patch (10.58 KB, patch)
2012-05-07 16:11 PDT, Enrica Casucci
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Enrica Casucci 2012-05-07 16:04:01 PDT
* 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>
Comment 1 Enrica Casucci 2012-05-07 16:11:07 PDT
Created attachment 140615 [details]
Patch
Comment 2 Alexey Proskuryakov 2012-05-07 16:14:28 PDT
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.
Comment 3 Enrica Casucci 2012-05-07 16:19:19 PDT
(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.
Comment 4 Enrica Casucci 2012-05-07 16:27:36 PDT
(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.
Comment 5 Ryosuke Niwa 2012-05-10 01:17:37 PDT
Closing the bug since the patch appears to have been landed in http://trac.webkit.org/changeset/116368.