Bug 87615 - REGRESSION(r108758): Can't edit <input> elements with :first-letter
Summary: REGRESSION(r108758): Can't edit <input> elements with :first-letter
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Hajime Morrita
URL:
Keywords:
: 86405 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-05-27 22:31 PDT by Hajime Morrita
Modified: 2013-05-08 06:39 PDT (History)
6 users (show)

See Also:


Attachments
Patch (8.49 KB, patch)
2012-05-27 22:38 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (9.36 KB, patch)
2012-05-27 23:29 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (10.88 KB, patch)
2012-05-27 23:39 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (8.82 KB, patch)
2012-05-28 01:03 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hajime Morrita 2012-05-27 22:31:18 PDT
Reported at http://crbug.com/129313
Comment 1 Hajime Morrita 2012-05-27 22:38:26 PDT
Created attachment 144270 [details]
Patch
Comment 2 Kent Tamura 2012-05-27 23:02:14 PDT
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().
Comment 3 Hajime Morrita 2012-05-27 23:29:05 PDT
Created attachment 144277 [details]
Patch
Comment 4 Hajime Morrita 2012-05-27 23:29:56 PDT
Kent-san, thanks for taking a look. The updated patch looks much simpler.
Comment 5 Kent Tamura 2012-05-27 23:31:47 PDT
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.
Comment 6 Hajime Morrita 2012-05-27 23:39:18 PDT
Created attachment 144279 [details]
Patch
Comment 7 Hajime Morrita 2012-05-27 23:40:16 PDT
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.
Comment 8 Kent Tamura 2012-05-27 23:50:06 PDT
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?
Comment 9 Kent Tamura 2012-05-27 23:51:34 PDT
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 .
Comment 10 Hajime Morrita 2012-05-28 00:35:15 PDT
(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.
Comment 11 Kent Tamura 2012-05-28 00:39:14 PDT
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.
Comment 12 Hajime Morrita 2012-05-28 01:03:48 PDT
Created attachment 144294 [details]
Patch
Comment 13 Hajime Morrita 2012-05-28 01:06:16 PDT
(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.
Comment 14 Kent Tamura 2012-05-28 02:03:45 PDT
Comment on attachment 144294 [details]
Patch

ok
Comment 15 WebKit Review Bot 2012-05-28 16:53:05 PDT
Comment on attachment 144294 [details]
Patch

Clearing flags on attachment: 144294

Committed r118711: <http://trac.webkit.org/changeset/118711>
Comment 16 WebKit Review Bot 2012-05-28 16:53:11 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 Raphael Kubo da Costa (:rakuco) 2013-05-08 06:39:02 PDT
*** Bug 86405 has been marked as a duplicate of this bug. ***