[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 #9 from Ryosuke Niwa <rniwa at webkit.org>  2011-06-03 20:58:30 PST ---
(In reply to comment #8)
> (In reply to comment #6) 
> > This logic is likely wrong as it doesn't take invisible nodes into account.
> 
> You're right that I didn't take invisible nodes into account at all when writing this code. By invisible, do you mean display:none or something else? Is there a case I should add as a layout test?

Yes. display: none and text nodes with collapsible whitespace, etc... In general, we need to be consistent in how we deal with these invisible nodes and the best way to do is to rely on VisiblePosition.

> > It worries me to no end that we're adding a brand new function for dealing with this very specific case whereas splitTreeToNode is a well-understood function and used in many places.
> 
> At the same time, complicating the arguments and adding one-off functionality to a simple, well-understood function used in many places seems quite dangerous, too. When I modify the function to take a position, I'll still need to answer this question about whether the position should be split, and make sure I don't introduce bugs with invisible nodes or anything else. Am I missing a less complicated way to fix this bug?

By splitting the position, do you mean splitting a text node which is the anchor node of a Position (where AnchorType is PositionIsOffsetInAnchor)?  We should be doing that iff we can come up with a test case where that is necessary.  As far as I know, we deal with that in callers.  But maybe it's better to encapsulate all of that in splitTreeToNode instead of imposing implicit pre-condition like that.

> Also, it took me a bit of extra time to understand what splitTreeToNode() is supposed to do because there wasn't a unit testing framework that allowed me to pass in arbitrary nodes/dom trees and look at the output. Do you think it would be worth trying to add common editing methods like this to window.internals so that we can test them independently? (Or maybe I'm missing some existing way to do this?)

I'm not so sure because that'll require us adding a custom editing command for splitTreeToNode since you can't call splitTreeToNode without instantiating a CompositeEditCommand.

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