[Webkit-unassigned] [Bug 62621] Span ID duplicated when pressing enter at beginning of span

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=62621





--- Comment #1 from Annie Sullivan <sullivan at chromium.org>  2011-06-14 15:38:44 PST ---
Ryosuke, in comment #7 of bug 61594, you suggested that before fixing this issue, we should merge InsertParagraphSeparatorCommand::cloneHierarchyUnderNewBlock() with CompositeEditCommand::cloneParagraphUnderNewElement() since they are very similar functions. I started looking into this, but several differences in functionality tripped me up:

1. The InsertParagraphSeparator version actually operates as two functions paired together: getAncestorsInsideBlock() collects the ancestors and then cloneHierarchyUnderNewBlock() clones them. There doesn't appear to be much reason for this, so it should be okay to change.

2. For listing the ancestors, getAncestorsInsideBlock takes an insertion Node and an outer block Element. It excludes both these nodes from the list of ancestors, only appending the elements between them. By contrast, cloneParagraphUnderNewElement() takes a start Position and an outer Node, and includes the position.deprecatedNode() and the outer node in the list of ancestors. Swapping between using positions and using nodes, counting ancestors differently, and keeping lists of Nodes instead of Elements is a bigger change in behavior. We also end up with another dependency on deprecatedNode.

3. cloneParagraphUnderNewElement() has no return value. cloneHierarchyUnderNewBlock() returns the deepest Element in the ancestor list that was cloned. We'd need to add this functionality into cloneParagraphUnderNewElement(), but it seems like an arbitrary return value to add.

Do you still think that merging these functions is the right thing to do? I am working on a patch to do this, but because of the differences in functionality I'm having a lot of trouble getting the arguments to cloneParagraphUnderNewElement correct. Each time I fix a layout test, a different layout test breaks with a slightly different problem.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.