Bug 142880

Summary: REGRESSION (r109593): Clicking after last inline element could cause a crash.
Product: WebKit Reporter: Enrica Casucci <enrica>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, buildbot, commit-queue, esprehn+autocc, glenn, kondapallykalyan, mmaxfield, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Repro case
none
Patch
none
Patch2
rniwa: review+, buildbot: commit-queue-
Archive of layout-test-results from ews101 for mac-mavericks
none
Archive of layout-test-results from ews107 for mac-mavericks-wk2 none

Description Enrica Casucci 2015-03-19 15:08:59 PDT
Created attachment 249063 [details]
Repro case

Run the attached test case and click after the end of the line containing the link.
Crash.
Comment 1 Enrica Casucci 2015-03-19 15:15:29 PDT
Created attachment 249065 [details]
Patch
Comment 2 WebKit Commit Bot 2015-03-19 15:17:31 PDT
Attachment 249065 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RootInlineBox.cpp:766:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/rendering/RootInlineBox.cpp:771:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 2 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Enrica Casucci 2015-03-19 15:51:32 PDT
Created attachment 249068 [details]
Patch2

Removing change to TestExpectations.
Comment 4 WebKit Commit Bot 2015-03-19 15:53:08 PDT
Attachment 249068 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RootInlineBox.cpp:766:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/rendering/RootInlineBox.cpp:771:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 2 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Build Bot 2015-03-19 16:08:56 PDT
Comment on attachment 249068 [details]
Patch2

Attachment 249068 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5923792496885760

New failing tests:
editing/selection/click-after-last-inline-crash.html
Comment 6 Build Bot 2015-03-19 16:09:00 PDT
Created attachment 249069 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 7 Ryosuke Niwa 2015-03-19 16:15:15 PDT
Comment on attachment 249068 [details]
Patch2

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

r=me provided you address the following comments.

> Source/WebCore/rendering/RootInlineBox.cpp:766
> -    if (leftPosition <= firstLeaf->logicalLeft() && !firstLeaf->renderer().isListMarker() && (!onlyEditableLeaves || isEditableLeaf(firstLeaf)))
> +    if (/*firstLeaf &&*/ leftPosition <= firstLeaf->logicalLeft() && !firstLeaf->renderer().isListMarker() && (!onlyEditableLeaves || isEditableLeaf(firstLeaf)))

Surely, you didn't mean to comment out this code?

> LayoutTests/editing/selection/click-after-last-inline-crash.html:1
> +<html>

Missing DOCTYPE.

> LayoutTests/editing/selection/click-after-last-inline-crash.html:16
> +    var testElement = document.getElementById('test');
> +    eventSender.mouseMoveTo(testElement.offsetLeft + testElement.offsetWidth + 50, testElement.offsetTop + 5);
> +    eventSender.mouseDown();
> +    eventSender.mouseUp();

Please wrap this code inside else.
Can we spit out "PASS" after running eventSender?
Comment 8 Build Bot 2015-03-19 16:16:07 PDT
Comment on attachment 249068 [details]
Patch2

Attachment 249068 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5360842543464448

New failing tests:
editing/selection/click-after-last-inline-crash.html
Comment 9 Build Bot 2015-03-19 16:16:11 PDT
Created attachment 249070 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 10 Enrica Casucci 2015-03-19 16:26:17 PDT
(In reply to comment #7)
> Comment on attachment 249068 [details]
> Patch2
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249068&action=review
> 
> r=me provided you address the following comments.
> 
> > Source/WebCore/rendering/RootInlineBox.cpp:766
> > -    if (leftPosition <= firstLeaf->logicalLeft() && !firstLeaf->renderer().isListMarker() && (!onlyEditableLeaves || isEditableLeaf(firstLeaf)))
> > +    if (/*firstLeaf &&*/ leftPosition <= firstLeaf->logicalLeft() && !firstLeaf->renderer().isListMarker() && (!onlyEditableLeaves || isEditableLeaf(firstLeaf)))
> 
> Surely, you didn't mean to comment out this code?
> 
> > LayoutTests/editing/selection/click-after-last-inline-crash.html:1
> > +<html>
> 
> Missing DOCTYPE.
> 
> > LayoutTests/editing/selection/click-after-last-inline-crash.html:16
> > +    var testElement = document.getElementById('test');
> > +    eventSender.mouseMoveTo(testElement.offsetLeft + testElement.offsetWidth + 50, testElement.offsetTop + 5);
> > +    eventSender.mouseDown();
> > +    eventSender.mouseUp();
> 
> Please wrap this code inside else.
> Can we spit out "PASS" after running eventSender?
My brain is out today.
Comment 11 Enrica Casucci 2015-03-19 17:48:46 PDT
Committed revision 181773.