Bug 87615 – REGRESSION(r108758): Can't edit <input> elements with :first-letter

• Component: Forms
• Status: RESOLVED FIXED

[Hajime Morrita]

Reported at http://crbug.com/129313

[Hajime Morrita]

Created attachment 144270 [details]
Patch

[Kent Tamura]

Comment on attachment 144270 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=144270&action=review

> Source/WebCore/rendering/RenderBlock.h:748
> +    virtual bool canHaveGeneratedChildren() const { return true; }
> +    static bool canHaveGeneratedChildren(RenderObject*);

Existence of static canHaveGeneratedChildren() and virtual canHaveGeneratedChildren() is confusing.
IMO, we should have "virtual bool RenderObject::canHaveGeneratedChildren()", of which default implementation is { return canHaveChildren(); }, and remove RenderBlock::canHaveGeneratedChildren().

[Hajime Morrita]

Created attachment 144277 [details]
Patch

[Hajime Morrita]

Kent-san, thanks for taking a look. The updated patch looks much simpler.

[Kent Tamura]

Comment on attachment 144277 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=144277&action=review

> LayoutTests/ChangeLog:9
> +        * fast/forms/input-first-letter-edit-expected.html: Added.
> +        * fast/forms/input-first-letter-edit.html: Added.

These files are not in the patch.

[Hajime Morrita]

Created attachment 144279 [details]
Patch

[Hajime Morrita]

Comment on attachment 144277 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=144277&action=review

>> LayoutTests/ChangeLog:9
>> +        * fast/forms/input-first-letter-edit.html: Added.
> 
> These files are not in the patch.

Oops. git reset dropped them. I'm sorry for disturbing. 
Updated the patch again.

[Kent Tamura]

Comment on attachment 144279 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=144279&action=review

> Source/WebCore/rendering/RenderBlock.cpp:5994
> -static inline RenderObject* findFirstLetterBlock(RenderBlock* start)
> +inline RenderObject* RenderBlock::findFirstLetterBlock()
>  {
> -    RenderObject* firstLetterBlock = start;
> +    RenderObject* firstLetterBlock = this;
>      while (true) {

Are these changes needed?

[Kent Tamura]

Comment on attachment 144279 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=144279&action=review

> LayoutTests/ChangeLog:9
> +        * fast/forms/input-first-letter-edit-expected.html: Added.
> +        * fast/forms/input-first-letter-edit.html: Added.

We had better add <input> with :before/:after .

[Hajime Morrita]

(In reply to comment #8)
> (From update of attachment 144279 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=144279&action=review
> 
> > Source/WebCore/rendering/RenderBlock.cpp:5994
> > -static inline RenderObject* findFirstLetterBlock(RenderBlock* start)
> > +inline RenderObject* RenderBlock::findFirstLetterBlock()
> >  {
> > -    RenderObject* firstLetterBlock = start;
> > +    RenderObject* firstLetterBlock = this;
> >      while (true) {
> 
> Are these changes needed?
Yes, I do't want to make canHaveGeneratedChildren() public.
It need to be accessed from a member function.

[Kent Tamura]

Comment on attachment 144279 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=144279&action=review

>>> Source/WebCore/rendering/RenderBlock.cpp:5994
>>>      while (true) {
>> 
>> Are these changes needed?
> 
> Yes, I do't want to make canHaveGeneratedChildren() public.
> It need to be accessed from a member function.

RenderObject::canHaveGeneratedChildren() is now public.

[Hajime Morrita]

Created attachment 144294 [details]
Patch

[Hajime Morrita]

(In reply to comment #11)
> (From update of attachment 144279 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=144279&action=review
> 
> >>> Source/WebCore/rendering/RenderBlock.cpp:5994
> >>>      while (true) {
> >> 
> >> Are these changes needed?
> > 
> > Yes, I do't want to make canHaveGeneratedChildren() public.
> > It need to be accessed from a member function.
> 
> RenderObject::canHaveGeneratedChildren() is now public.
RIght.  Moved back findFirstLetterBlock() to a static function.

On testing after/before, I'll address it on Bug 87630.

[Kent Tamura]

Comment on attachment 144294 [details]
Patch

ok

[WebKit Review Bot]

Comment on attachment 144294 [details]
Patch

Clearing flags on attachment: 144294

Committed r118711: <http://trac.webkit.org/changeset/118711>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Raphael Kubo da Costa (:rakuco)]

*** Bug 86405 has been marked as a duplicate of this bug. ***