Bug 142880

Summary:

REGRESSION (r109593): Clicking after last inline element could cause a crash.

Product:

WebKit

Reporter:

Enrica Casucci <enrica>

Component:

Layout and Rendering

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, buildbot, commit-queue, esprehn+autocc, glenn, kondapallykalyan, mmaxfield, rniwa

Priority:

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

Enrica Casucci

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

Attachments
Repro case (271 bytes, text/html) 2015-03-19 15:08 PDT, Enrica Casucci	no flags	Details
Patch (4.99 KB, patch) 2015-03-19 15:15 PDT, Enrica Casucci	no flags	Details Formatted Diff Diff
Patch2 (4.33 KB, patch) 2015-03-19 15:51 PDT, Enrica Casucci	rniwa: review+ buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews101 for mac-mavericks (604.29 KB, application/zip) 2015-03-19 16:09 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews107 for mac-mavericks-wk2 (634.09 KB, application/zip) 2015-03-19 16:16 PDT, Build Bot	no flags	Details
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Enrica Casucci

Comment 1 2015-03-19 15:15:29 PDT

Created attachment 249065 [details] Patch

WebKit Commit Bot

Comment 2 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.

Enrica Casucci

Comment 3 2015-03-19 15:51:32 PDT

Created attachment 249068 [details] Patch2 Removing change to TestExpectations.

WebKit Commit Bot

Comment 4 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.

Build Bot

Comment 5 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

Build Bot

Comment 6 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

Ryosuke Niwa

Comment 7 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?

Build Bot

Comment 8 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

Build Bot

Comment 9 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

Enrica Casucci

Comment 10 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.

Enrica Casucci

Comment 11 2015-03-19 17:48:46 PDT

Committed revision 181773.

Note You need to log in before you can comment on or make changes to this bug.