Summary: | REGRESSION (r101575): Chinese input is broken when composing mail in iCloud using Safari | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Enrica Casucci <enrica> | ||||
Component: | HTML Editing | Assignee: | Enrica Casucci <enrica> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | rniwa | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Mac | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Enrica Casucci
2012-05-07 16:04:01 PDT
Created attachment 140615 [details]
Patch
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. (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. (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. Closing the bug since the patch appears to have been landed in http://trac.webkit.org/changeset/116368. |