If you have a div that is marked contentEditable, but it does not contain some visible characters in it -- e.g. it's empty of just <p /> in it -- then you can't actually get an insertion point to edit text in the field Steps to reproduce: See attached test case.
Created attachment 2965 [details] Simple test case showing inability to start editing div when it's empty
confirmed on 10.3.9
Confirmed with ToT WebKit.
The VisiblePosition created and returned in positionForCoordinates is outside the editable div that was clicked. I thought that this bug would be fixed by my patch for 5354 but it isn't. In the test case, the div has a renderer and height of 22, and has rendered child, the p. The p doesn't have a height. Neither [p, 0] or [div, 0] return true from isCandidate(), so visibleposition::init leaves the div and uses the next visibleposition, which is the start of the next paragraph. I think the fix should be in isCandidate().
What exactly is correct behaviour for the second case where there is a <p /> in the div? Should the insertion point be placed before the <p />, in the <p></p>, or after the <p />? I've had a bit of a play with this, but I'm not sure which is right. So far, I've got the first two cases to work, but with graphics bugs.
Created attachment 4732 [details] editable div test: empty vs. white space Additional test case contrasting an editable div that is truly empty with an editable div that has some whitespace in it
(In reply to comment #5) A good question that I don't have an answer to. My main concern in this bug report is how to edit a div when just an empty paragraph or nbsp is in it (which, after some more investigation, is also a problem if you have *any* "blank" content such as a space or newline in your source). There was some discussion on this on #webkit yesterday, mostly about whether you can get a carat if the element has no dimensions. With an empty <p />, it has no dimensions -- though the enclosing <div> does. I noticed that if you just specify a div to be contentEditable, it seems to pick up some automatic min-height behavior (see my second attachment for an example, vs. my first attachment, where I specified explicit min-heights). Clearly the tricky case is what to do if you have a no-dimension entity like the middle paragraph here: <p>first paragraph</p> <p /> <p>third paragraph</p>
Created attachment 4735 [details] Patch for a contentEditable div containing only whitespace This patch builds on the one for 5354, and for a div with no renderable children (contains only whitespace, no <p />, etc), puts the insertion pointer at the start of the div (before the #text element). I don't know, but it might be preferable to set the insertion pointer in the #text element.
Created attachment 4892 [details] patch 2 This sets the cursor at the div node for both the whitespace and <p /> situations. The latter involves a rendering bug that appears to be a regression from the WebKit in 10.4.3
Comment on attachment 4892 [details] patch 2 I think Justin should review this.
Comment on attachment 4892 [details] patch 2 This patch is great! Here are some comments: Since the next/prevIsInOriginalBlock booleans now include a check for identity with the originalBlock, it no longer makes sense for them to be named "IsIn": in what sense is A "in" A? Instead, you could do: bool nextIsOutsideOriginalBlock = !next.node()->isAncestor(originalBlock) && next.node() != originalBlock bool prevIsOutsideOriginalBlock = !prev.node()->isAncestor(originalBlock) && prev.node() != originalBlock if (nextIsOutsideOriginalBlock && !prevIsOutsideOriginalBlock && ... etc. Also, I would put hasRenderedChildrenWithHeight into a function, it would make isCandidate a little easier to read.
Created attachment 4900 [details] patch 3a Addressing comments. An additional function was added to RenderObject, hasChildrenWithHeight. I'll add another patch that doesn't modify RenderObject if adding a member function is a Bad Thing.
Created attachment 4901 [details] patch 3b Second proposed patch.
Comment on attachment 4901 [details] patch 3b Made some minor tweaks and landed this.