[Webkit-unassigned] [Bug 30116] WebCore::InsertLineBreakCommand::shouldUseBreakElement ReadAV at NULL

[bugzilla-daemon at webkit.org]

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





--- Comment #20 from Tony Chang (Google) <tony at chromium.org>  2010-06-09 18:45:30 PST ---
Ojan, I can't tell if you're reviewing the patches since you're not changing the review flag.  Inline comments on that patch are more helpful than just responding to the comments.


(In reply to comment #18)
> Before this patch, pos pointed to the deepEquivalent (canonicalized node). After this patch, it just points to m_start, which may or may not be canonicalized. Right?

Sure, but what canonicalization needs to happen?  I.e., do you have an example test case that this breaks?  Since this is just plain text, I'm not sure we need to canonicalize here.

> > VisiblePosition::canonicalPosition returns null because PositionIterator checks for visibility.  Changing this would be a big change which seems beyond the scope of this bug.  But maybe you're asking about only changing visibleStart some of the time?  That also seems tricky to implement (e.g. what happens if the rootEditableElement is visible but some child editable node is hidden)?
> 
> I think it's OK to special-case the rootEditableElement here. If a child isn't visible, the typing will still go through just fine. It just won't go inside the visibility:hidden element. It'll go in one of it's siblings/parents/cousins. I think it would be sufficient for the visibility check in PositionIterator::isCandidate and Position::isCandidate to not return false there if the renderer is the renderer for the rootEditableElement, but I'm not sure.

I mean something like this:
<div contenteditable>
<p>foo bar</p>
<p id="to-be-made-hidden"> | </p>
</div>

If the cursor is at |, and the keydown event makes "to-be-made-hidden" hidden, what should happen?  Currently, if the node with the cursor disappears, the selection is lost.  I think it would be weird if the cursor jumped to the foo bar paragraph.

Anyway, shouldn't this be discussed on a different bug?

> This looks like we'll do the wrong thing (use a paragraph instead of a BR) if you hit enter inside a table that's not visible. That's much better than crashing though. Maybe add a FIXME pointing to bug 40342?

done.

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