Undoing a indent removes text instead of it's formatting. Reproduces in both Gmail and Google Docs with Webkit Nightly r47686
* STEPS TO REPRODUCE
1. Launch Webkit Nightly r47686 and sign into your gmail account.
2. Compose a new message and type hello in the message body
3. Click the indent more text icon to indent this text
4. After indenting, select Edit - Undo Indent. Notice word is removed rather removing the indent. Also focus is removed from message body as well.
Using webkit nightly builds, I was able to isolate to a range where the regression occurred :
r46126 (work ) - r46193 (fails )
I tracked the regression to http://trac.webkit.org/changeset/46143
You must mean http://trac.webkit.org/changeset/46142.
Outdent and Command+z work. I don't quite understand why Command+z works and Edit->Undo doesn't.
I suspect that docs intercepts command+z, but they can't intercept "undo" from the menu.
Created attachment 40957 [details]
This patch fixes the bug and provides a test case for bug 23995.
Comment on attachment 40957 [details]
Clearing flags on attachment: 40957
Committed r49405: <http://trac.webkit.org/changeset/49405>
All reviewed patches have been landed. Closing bug.
(In reply to comment #8)
> Created an attachment (id=40957) [details]
> This patch fixes the bug and provides a test case for bug 23995.
This patch reverted my changes. And I think that's reasonable because I didn't present any reasonable solution to it on time, but now all my test cases are useless (they are not testing what it is supposed to be testing). Could you file a bug for it or explain why those changes are acceptable?
I had a long chat on IRC with Ryosuke and I explained the reasons for the changes.
I've also pointed out that the solution he implemented was not correct because it was breaking the undo mechanism.
The undo mechanism relies on commands being built on a stack to perform the undo. The previous implementation was only performing DOM operations, without placing commands on the stack.
I also pointed out that removing moveParagraph was not the solution to the wrong selection problem, because that issue was caused by a missing call to updateLayout that made TextIterator fail to retrieve the correct positions for the selection.
As far as the other changes that were part of the previous patch, even though I do see the point of not stripping out divs that have an id, I don't think that it was the primary goal and can be addressed in another patch. Furthermore, that solutions creates other selection problems.
In my opinion this bug should be marked as fixed.
I agree that this bug should be resolved. This change fixed the regression described here, while retaining good indentation behavior. Let's open new bugs about any other related issues we discover.