Open the attached test case
Click after the 'o' in "Francisco" to start editing
Delete removes two characters at a time.
<rdar://problem/9299050> Delete removes two characters at a time in a container that has :first-letter applied to it
<div>Graffiti in San Francisco</div>
Created attachment 90058 [details]
This is more severe than I thought. Characters don't go where you insert them and WebKit hangs when you try to move the caret before the first letter of the container.
There might be similar old-4digit bug but I dont' remember.
Kent-san, do you remember that?
See also: bug 26442, bug 26443, bug 43830
(In reply to comment #3)
> There might be similar old-4digit bug but I dont' remember.
> Kent-san, do you remember that?
I'm not sure this bug is same as bugs Alexey listed.
I'll make a meta bug for :first-letter.
Re-titling back to track only the specific issue reported originally, now that we have a separate meta bug.
Created attachment 144283 [details]
The string contains 25 characters. With current webkit design when there is a first letter preset it uses RenderTextFragment. For this string it creates 2 RenderTextFragment as follows:-
1. RenderTextFragments with first letter, start offset:0 and endoffset/length:1
2. RenderTextFragments with remaining letter, start offset: 1 and endoffset/length:24
Now when we place the cursor at the end , the offsets are as follows:-
- offset 25 in original string and offset 24 in RenderTextFragment for remaining text.
Now when we delete the text, it uses the RenderTextFragmnets offsets to calclulate the new offset. Since the current offset for it is 24 it calculates the new offsets as 23.
Now while deleting the characters it uses the original string (with 25 characters), and the new offsets calculated using RenderTextFragment (which is 23).
The code that is used to delete is "DeleteSelectionCommand::handleGeneralDelete".
In this method it uses the offset to determine the length as follows:-
deleteTextFromNode(text, startOffset, m_downstreamEnd.deprecatedEditingOffset() - startOffset);
So since the new offset (calculated using the RenderTextFragment) is 23 and the value of m_downstreamEnd.deprecatedEditingOffset (which is same as the length of original string) is 25, the number of elements to be removed is calculated as 2. Hence it deletes 2 characters.
So there is a mismatch in calculation of offsets when first letter is involved.So if we add the start offset value of RenderTextFragment to the offset that is calculated in deletes one character. A reference patch is attached.
Please provide your feedback.
As I have mentioned in my previous analysis, the issue is caused by using renderTextFragments offsets while calculating the offsets position. I have been working on to find some solution and I have below ideas:
1. We should make RenderTextFragment child of text renderer (also suggested by Levi in bug 45174).
2. We should implement first-letter in the same way first-line is implemented.
- I checked the selector spec and I think it didn't mandate in specific internal implementation (http://www.w3.org/TR/css3-selectors/). Please correct me if my understanding is wrong.
I am trying to figure out other options. Request you to provide feedback on the above mentioned options.
We can't implement first-letter like we implement first-line so the only option is 1. Also see the bug I'cced you about how we should have regular children under RenderTextFragment.
(In reply to comment #9)
> We can't implement first-letter like we implement first-line so the only option is 1.
Thanks for the feedback. I will work on this.
Also see the bug I'cced you about how we should have regular children under RenderTextFragment.
are you talking of bug 63368? yes, I am looking at.
(In reply to comment #11)
> (In reply to comment #9)
> > We can't implement first-letter like we implement first-line so the only option is 1.
> Thanks for the feedback. I will work on this.
also should I post my patches here or on bug 63368?
bug 63368 seems more relevent for the RenderText/RenderTextFragment change.
(In reply to comment #11)
> are you talking of bug 63368? yes, I am looking at.
(In reply to comment #12)
> also should I post my patches here or on bug 63368?
> bug 63368 seems more relevent for the RenderText/RenderTextFragment change.