Created attachment 66491 [details]
WebPage with ::first-letter and contenteditable applied.
Steps to reproduce crash:
1. Open attachment in Webkit, Safari or Chrome.
2. Click on the right of the 'O'.
3. Type a single space.
4. Press the left arrow key twice.
5. Now, it is too late. The browser (or the tab, in Chrome)
is no longer responding. It uses all the CPU until killed.
Hangs in VisiblePosition::leftVisuallyDistinctCandidate().
This no longer hangs on TOT WebKit in Safari or Chrome. Instead, typing space causes all the text to be lost. Also, hitting the left arrow key causes the caret to keep jumping left and right to between the 'K' and 'O' and after the 'O', but never actually getting before the 'K'.
Created attachment 73413 [details]
Removing leading space leads to hang
Removing the leading space before the 'K' actually causes the hang to once-again manifest in TOT WebKit.
The first-letter implementation seems to break assumptions made throughout WebKit. In our example, we end up with a render tree that looks something like this:
-- RenderTextFragment "K"
- RenderTextFragment "O"
where "O" is considered the renderer for the DOM text node "KO". Breaking the RenderText into two RenderTextFragments who's first common ancestor is a renderer for a different node causes the first letter to be "lost" in visible units. It seems like we need to nest the RenderTextFragments and the RenderInline that contains the first-letter style inside of a new RenderObject that is considered the DOM node's renderer.
I started working on a fix for this but unfortunately reached a bit of a head... Position and VisiblePosition code needs to be made entirely too aware of the atypical Renderer layout of text nodes with first-letter content for my fix to be clean.
First of all, it seems that Position shouldn't be aware of Renderer code, and should be able to get all it needs from the DOM. Furthermore, IMHO VisiblePosition could be cleaned up to push strange behavior like this into Renderer's own implementation, instead of trying to handle all cases in one class.
Created attachment 74042 [details]
This patch solves a number of issues surrounding first-letter:
- leftVisuallyDistinctCandidate no longer hangs
- the caret can correctly be positioned in first-letter content
- typing at the end of an editable field with first-letter content no longer replaces the final character (Bug 26442)
There are still serious issues with selection and contenteditable, all stemming from the fundamental break from the rendering norm that first-letter presents by having part of a text node's rendered content in a fully separate renderer than the one it directly owns. Solving the remaining bugs involves making more parts of the code aware of this specific render-tree structure, which makes me uncomfortable.
I believe that having the text node's renderer be the parent (or grand-parent) of *all* associated RenderTextFragments would solve many of these problems, but I'll defer to those who better understand WebKit rendering. Feedback is very appreciated. I think this solution is not ideal.
I am currently working on html editing issues and found that with current implentation, there is a gap in how the offsets are calculated using RenderTextFragments (bug 58800). So I think there is a need to change the current working of RenderTextFragments.
Levi Weintraub: If you are not working on the issue, then I would like to take the issue forward.
(In reply to comment #8)
> I am currently working on html editing issues and found that with current implentation, there is a gap in how the offsets are calculated using RenderTextFragments (bug 58800). So I think there is a need to change the current working of RenderTextFragments.
> Levi Weintraub: If you are not working on the issue, then I would like to take the issue forward.
I'd love to have your patch. I haven't been thinking about this problem for quite some time and would be happy to take a look at any patches you have :)
(In reply to comment #9)
> Hi Antaryami!
> I'd love to have your patch. I haven't been thinking about this problem for quite some time and would be happy to take a look at any patches you have :)
I don't have a patch ready with me. But while working on editing issues I also found same issue that you have analyzed. There is an issue how the offsets are calculated and used using RenderTextFragment.
I am working on to make editing works with first letter.
I will also appreciate any direction/guide from you.