Bug 30955 – REGRESSION: In Mail, Undo does not restore some characters I have deleted at the end of a line

• Component: HTML Editing
• Status: RESOLVED FIXED

[Enrica Casucci]

* STEPS TO REPRODUCE
1. Create a new message in any Webmail application
2. Type the following without using backspace to correct any words (the exact words are not important)

one two three four five six seven

3. Delete the last word or two using alt+shift+delete without moving the insertion point (without using arrow keys), so you have this:

one two three four five

4. Undo (CMD+Z)

* RESULTS
the entire content is erased
* EXPECTED
the delete word is restored

[Enrica Casucci]

Created attachment 42215 [details]
Patch

[Darin Adler]

Comment on attachment 42215 [details]
Patch

> -    if (isOpenForMoreTypingCommand(lastEditCommand)) {
> +    if (!killRing && isOpenForMoreTypingCommand(lastEditCommand)) {

It's not correct to use the killRing boolean for this. That has a specific meaning, and it just happens to be set in the appropriate cases. I would like to see a new patch that does this in some logically consistent way.

One possibility would be to continue to use this boolean and rename it to reflect its new meaning.

Another possibility would be to add some other argument or mechanism for controlling whether new keys are added to the existing typing.

[Adele Peterson]

Comment on attachment 42215 [details]
Patch

One more comment- I think you can use layoutTestController.dumpAsText() in this test.  I don't think you need the full RenderTree in the results.

[Enrica Casucci]

Created attachment 42223 [details]
Patch2

Per Darin's suggestions, we don't the killRing flag, but instead we rely on the TextGranularity to decide whether to start a new command or not.
I've also modified the unit test per Adele's suggestions.

[Darin Adler]

Comment on attachment 42223 [details]
Patch2

> +    if (granularity ==  CharacterGranularity && isOpenForMoreTypingCommand(lastEditCommand)) {

There's an extra space here before the "C".

r=me

[Enrica Casucci]

Created attachment 42226 [details]
Patch3

New patch removing extra blank.

[Adele Peterson]

Committed revision 50358