[Webkit-unassigned] [Bug 61594] REGRESSION: Hitting enter in the middle of this span causes the cursor to go to the end of the span

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from Ryosuke Niwa <rniwa at webkit.org>  2011-06-06 13:32:20 PST ---
(In reply to comment #10)
> Yes, by splitting the position, I mean if the position is in the middle of a text node, it seems logical that splitTreeToPosition() would split the text node so that the tree is split exactly at the given position. Right now, all the callers do the splitting themselves, since the function used to only split to node boundaries.

Another concern I have is that callers of splitTree wouldn't be aware of the fact text node has been split.  So if they kept some positions anchored at the text node or have some assumptions about DOM structure around the text node, they might get affected by this change.  

> I agree that it's not a great idea to add code if no one uses it yet. But I think that since the new function will take a position, it will be confusing to callers who aren't familiar with the history of the editing code why callers are expected to do any actual splitting of text nodes at the position. Not sure how this type of problem is usually handled--a comment above the function?

I'd rather rename the function or change the function behavior than adding a comment. In general, we favor refactoring over adding comments.

> Also, I'm still not clear on what the declaration of the new function should be:
> Should it be renamed to splitTreeToPosition?

I always thought ToNode didn't any value to the function name.  I'd try renaming it to splitTree.

> Should it take Position or a VisiblePosition?

Is there a benefit in this function knowing upstream/downstream?  VisiblePosition is an expensive object to instantiate so I'd start with Position unless there is a compelling reason.

> My understanding is that CompositeEditingCommand exists to allow shared code between various editing commands. I know in some codebases, making a UnitTestEditingCommand that derives from CompositeEditingCommand that we could use to write unit tests for all those functions would be a way to do this. Not sure about WebKit.

That'll require us creating a Document, DocumentLoader, etc... because CompositeEditCommand can't be instantiated without a document.  In the discussion we had in webkit-dev, the general consensus (as far as I understand it) was to try to add unit tests for things in wtf and other components that are isolated from the rest of WebKit.  And I don't think editing is such a component because it depends heavily on CSS, DOM, and rendering components.

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