Bug 58800

Summary: Delete removes two characters at a time in a container that has :first-letter applied to it
Product: WebKit Reporter: Justin Garcia <justin.garcia>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: antaryami.pandia, ap, enrica, igor.oliveira, morrita, rniwa, tkent
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 58911    
Attachments:
Description Flags
testcase
none
Reference Patch. none

Description Justin Garcia 2011-04-18 11:13:26 PDT
Open the attached test case
Click after the 'o' in "Francisco" to start editing
Start deleting 

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

<head>
<style>
div {
    font-size: 48px;
}
div:first-letter {
    color: blue;
}
</style>
</head>
<body contentEditable="true">
<div>Graffiti in San Francisco</div>
</body>
Comment 1 Justin Garcia 2011-04-18 11:13:58 PDT
Created attachment 90058 [details]
testcase
Comment 2 Justin Garcia 2011-04-18 11:26:18 PDT
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.
Comment 3 Hajime Morrita 2011-04-18 12:52:58 PDT
There might be similar old-4digit bug but I dont' remember.
Kent-san, do you remember that?
Comment 4 Alexey Proskuryakov 2011-04-18 14:41:56 PDT
See also: bug 26442, bug 26443, bug 43830
Comment 5 Kent Tamura 2011-04-19 11:33:32 PDT
(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.
Comment 6 Alexey Proskuryakov 2011-04-19 11:43:04 PDT
Re-titling back to track only the specific issue reported originally, now that we have a separate meta bug.
Comment 7 Antaryami Pandia (apandia) 2012-05-28 00:01:47 PDT
Created attachment 144283 [details]
Reference Patch.

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.
Comment 8 Antaryami Pandia (apandia) 2012-06-07 02:02:01 PDT
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.
Comment 9 Ryosuke Niwa 2012-06-07 18:10:58 PDT
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.
Comment 10 Ryosuke Niwa 2012-06-07 18:13:57 PDT
See https://bugs.webkit.org/show_bug.cgi?id=63368
Comment 11 Antaryami Pandia (apandia) 2012-06-07 19:52:30 PDT
(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.
Comment 12 Antaryami Pandia (apandia) 2012-06-07 20:10:53 PDT
(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.
Comment 13 Ryosuke Niwa 2012-06-07 21:12:05 PDT
(In reply to comment #11)
> are you talking of bug 63368? yes, I am looking at.

Yes.

(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.

Right.