Bug 26816 - Selection changed by indent
: Selection changed by indent
Status: RESOLVED FIXED
: WebKit
HTML Editing
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To:
:
:
: 27038
:
  Show dependency treegraph
 
Reported: 2009-06-29 14:40 PST by
Modified: 2009-07-21 15:25 PST (History)


Attachments
Refactors indentRegion further, and fixes the bug 26816 and 25317 (57.81 KB, patch)
2009-07-06 18:40 PST, Ryosuke Niwa
no flags Review Patch | Details | Formatted Diff | Diff
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 PST, Ryosuke Niwa
no flags Review Patch | Details | Formatted Diff | Diff
fixes the bug (42.28 KB, patch)
2009-07-08 00:55 PST, Ryosuke Niwa
no flags Review Patch | Details | Formatted Diff | Diff
change due to the resubmission for 27038 (42.23 KB, patch)
2009-07-08 16:41 PST, Ryosuke Niwa
eric: review+
Review Patch | Details | Formatted Diff | Diff
5th submission (45.71 KB, patch)
2009-07-19 23:32 PST, Ryosuke Niwa
eric: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-06-29 14:40:48 PST
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
------- Comment #1 From 2009-06-29 14:50:20 PST -------
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.
------- Comment #2 From 2009-07-06 18:40:12 PST -------
Created an attachment (id=32347) [details]
Refactors indentRegion further, and fixes the bug 26816 and 25317
------- Comment #3 From 2009-07-07 00:39:21 PST -------
(From update of attachment 32347 [details])
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. :(
------- Comment #4 From 2009-07-07 01:51:51 PST -------
Created an attachment (id=32367) [details]
Refactors indentRegion and fixes the bugs 26816 and 25317 (a minor change on the prototype of indentListItem)
------- Comment #5 From 2009-07-08 00:55:39 PST -------
Created an attachment (id=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.
------- Comment #6 From 2009-07-08 16:41:35 PST -------
Created an attachment (id=32487) [details]
change due to the resubmission for 27038
------- Comment #7 From 2009-07-14 15:09:24 PST -------
(From update of attachment 32487 [details])
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!
------- Comment #8 From 2009-07-19 23:32:28 PST -------
Created an attachment (id=33073) [details]
5th submission

The followup bug is filed as the bug 27441.
------- Comment #9 From 2009-07-20 14:58:05 PST -------
(From update of attachment 33073 [details])
Looks OK.
------- Comment #10 From 2009-07-21 15:25:34 PST -------
Landed in http://trac.webkit.org/changeset/46142