Bug 26816

Summary: Selection changed by indent
Product: WebKit Reporter: Julie Parent <jparent>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, justin.garcia, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on: 27038    
Bug Blocks:    
Attachments:
Description Flags
Refactors indentRegion further, and fixes the bug 26816 and 25317
none
Refactors indentRegion and fixes the bugs 26816 and 25317 (a minor change on the prototype of indentListItem)
none
fixes the bug
none
change due to the resubmission for 27038
eric: review+
5th submission eric: review+

Julie Parent
Reported 2009-06-29 14:40:48 PDT
This is a follow up from https://bugs.webkit.org/show_bug.cgi?id=21712, so just repeating those repro steps here. Repro steps: Go to http://www.mozilla.org/editor/midasdemo/ 1. Type one<enter>two<enter>three<enter>four 2. Select all, click on ordered list 3. Select "two" and "three" and click indent, forming a sublist. 4. Select "three" and click outdent, joining three back with the outer list. 5. Select "three" by double clicking and click indent, bringing three back into the sublist. Result: "three" is no longer selected, instead the last "e" from three is selected, and the "fou" in four on the next line is selected Expected result: Selection should be unchanged, "three" should still be selected. I think this was a result of http://trac.webkit.org/changeset/45316
Attachments
Refactors indentRegion further, and fixes the bug 26816 and 25317 (57.81 KB, patch)
2009-07-06 18:40 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Refactors indentRegion and fixes the bugs 26816 and 25317 (a minor change on the prototype of indentListItem) (57.80 KB, patch)
2009-07-07 01:51 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
fixes the bug (42.28 KB, patch)
2009-07-08 00:55 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
change due to the resubmission for 27038 (42.23 KB, patch)
2009-07-08 16:41 PDT, Ryosuke Niwa 		eric: review+
DetailsFormatted DiffDiff
5th submission (45.71 KB, patch)
2009-07-19 23:32 PDT, Ryosuke Niwa 		eric: review+
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2009-06-29 14:50:20 PDT
Yes, I have become aware of this problem right after submitting the patch. This is due to changeset 45316. The fact that indentRegion stores the index of selection and restores it later is to with selection being changed. I believe restoring selection is only needed because of what moveParagraph does. Right now, I'm trying to avoid using moveParagraph at all since this function is causing a lot of problems.
Ryosuke Niwa
Comment 2 2009-07-06 18:40:12 PDT
Created attachment 32347 [details] Refactors indentRegion further, and fixes the bug 26816 and 25317
Eric Seidel (no email)
Comment 3 2009-07-07 00:39:21 PDT
Comment on attachment 32347 [details] Refactors indentRegion further, and fixes the bug 26816 and 25317 This patch is rather large to review. I certainly can't do so tonight, and I think you'll have trouble finding a reviewer for a patch of this size. :(
Ryosuke Niwa
Comment 4 2009-07-07 01:51:51 PDT
Created attachment 32367 [details] Refactors indentRegion and fixes the bugs 26816 and 25317 (a minor change on the prototype of indentListItem)
Ryosuke Niwa
Comment 5 2009-07-08 00:55:39 PDT
Created attachment 32436 [details] fixes the bug This patch may be committed only after the patch for https://bugs.webkit.org/show_bug.cgi?id=27038 is committed.
Ryosuke Niwa
Comment 6 2009-07-08 16:41:35 PDT
Created attachment 32487 [details] change due to the resubmission for 27038
Eric Seidel (no email)
Comment 7 2009-07-14 15:09:24 PDT
Comment on attachment 32487 [details] change due to the resubmission for 27038 Should have a bug: + // FIXME: we need to deal with the case where there is no li (malformed HTML) + if (!selectedListItem->hasTagName(liTag)) + return false; 024 \ No newline at end of file Needs an arg name for the Node*: 59 void indentIntoBlockquote(const VisiblePosition&, const VisiblePosition&, RefPtr<Element>&, Node*); Looks fine otherwise. Thanks!
Ryosuke Niwa
Comment 8 2009-07-19 23:32:28 PDT
Created attachment 33073 [details] 5th submission The followup bug is filed as the bug 27441.
Eric Seidel (no email)
Comment 9 2009-07-20 14:58:05 PDT
Comment on attachment 33073 [details] 5th submission Looks OK.
Ryosuke Niwa
Comment 10 2009-07-21 15:25:34 PDT
Landed in http://trac.webkit.org/changeset/46142
Note You need to log in before you can comment on or make changes to this bug.