Bug 111894

Summary:

REGRESSION (r143313): Crash when selecting Hebrew and numbers in a list

Product:

WebKit

Reporter:

Yair Yogev <progame+wk>

Component:

HTML Editing

Assignee:

Arpita Bahuguna <arpitabahuguna>

Status:

RESOLVED FIXED

Severity:

Major

CC:

ap, arpitabahuguna, buildbot, dglazkov, eric, esprehn+autocc, ojan.autocc, ojan, playmobil, rniwa, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Test case	none
Test case (with correct encoding)	none
Patch	buildbot: commit-queue-
Archive of layout-test-results from gce-cr-linux-08 for chromium-linux-x86_64	none
Patch	none
Archive of layout-test-results from webkit-ews-04 for mac-future	none
Patch	none

Yair Yogev

Reported 2013-03-08 15:39:31 PST

Created attachment 192296 [details] Test case So far only tested Chromium and under Windows XP. Steps to repro: 1. Open the attached Test case. 2. Do as shown in this screen capture http://youtu.be/HSpnWFDehSE (that is- select the text from the right, all the way to the left) Regression Window: FAIL: Chromium 27.0.1418.0 (Developer Build 183371) WebKit 537.32 (@143340) OK: Chromium 27.0.1418.0 (Developer Build 183338) WebKit 537.32 (@143310) Since i couldn't find a reasonable candidate in the chromium changelog http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog.html?url=%2Ftrunk%2Fsrc&range=183338%3A183371&mode=html chances are it's something in the following WebKit changelog http://trac.webkit.org/log/?rev=143340&stop_rev=143310&verbose=on

Attachments
Test case (60 bytes, text/html) 2013-03-08 15:39 PST, Yair Yogev	no flags	Details
Test case (with correct encoding) (84 bytes, text/html) 2013-03-11 13:09 PDT, Yair Yogev	no flags	Details
Patch (4.73 KB, text/plain) 2013-03-27 04:52 PDT, Arpita Bahuguna	buildbot: commit-queue-	Details
Archive of layout-test-results from gce-cr-linux-08 for chromium-linux-x86_64 (1.98 MB, application/zip) 2013-03-27 06:11 PDT, WebKit Review Bot	no flags	Details
Patch (4.73 KB, patch) 2013-03-27 06:52 PDT, Arpita Bahuguna	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-04 for mac-future (779.56 KB, application/zip) 2013-03-27 22:07 PDT, Build Bot	no flags	Details
Patch (4.72 KB, patch) 2013-03-28 01:17 PDT, Arpita Bahuguna	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Yair Yogev

Comment 1 2013-03-08 15:48:23 PST

ojan/rniwa, any chance it's because of the fix in Bug 108053? it's the only thing the looks (somewhat) related to me in the changelog above. cross-posted in http://crbug.com/181198

Alexey Proskuryakov

Comment 2 2013-03-11 12:38:02 PDT

What charset should the test case be rendered with? I get a different rendering with iso-8859-8.

Yair Yogev

Comment 3 2013-03-11 13:09:07 PDT

Created attachment 192536 [details] Test case (with correct encoding) oops sorry, forgot about the encoding issue in bugzilla, should be OK now

Alexey Proskuryakov

Comment 4 2013-03-11 13:34:36 PDT

I cannot reproduce in Safari on Mac with a recent debug build.

Arpita Bahuguna

Comment 5 2013-03-11 18:10:18 PDT

Hi Yair, I am currently on travel and can work on this post 18th march, but as far as i could check right now, the localCaretRect() function itself is not being hit for this case. Perhaps am missing something, would need to check in further detail for that. But I was unable to reproduce this crash on my Safari debug build. Apologize if indeed this is a regression due to 108053.

Arpita Bahuguna

Comment 6 2013-03-27 04:13:30 PDT

Hi Ryosuke, Yair, This issue is indeed a regression due to r143313 (Bug #108053). I had earlier not carried out the selection step while trying to reproduce the issue, assuming it to be a page load crash. My sincere apologies for the same. I will shortly upload a patch for fixing this crash issue.

Yair Yogev

Comment 7 2013-03-27 04:24:41 PDT

Thanks, I'm glad you were able to reproduce the issue, in some rare occasion it's problematic (I'm not sure why...)

Arpita Bahuguna

Comment 8 2013-03-27 04:52:41 PDT

Created attachment 195278 [details] Patch

WebKit Review Bot

Comment 9 2013-03-27 06:10:58 PDT

Comment on attachment 195278 [details] Patch Attachment 195278 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17242537 New failing tests: editing/selection/5076323-3.html editing/selection/mixed-editability-9.html editing/deleting/4922367.html editing/selection/table-caret-1.html editing/inserting/insert-paragraph-01.html editing/selection/move-by-line-001.html editing/selection/5076323-2.html editing/selection/editable-non-editable-crash.html editing/selection/table-caret-3.html editing/selection/table-caret-2.html editing/selection/5131716-2.html

WebKit Review Bot

Comment 10 2013-03-27 06:11:04 PDT

Created attachment 195296 [details] Archive of layout-test-results from gce-cr-linux-08 for chromium-linux-x86_64 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-08 Port: chromium-linux-x86_64 Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04

Arpita Bahuguna

Comment 11 2013-03-27 06:52:32 PDT

Created attachment 195306 [details] Patch

Ryosuke Niwa

Comment 12 2013-03-27 09:15:56 PDT

Comment on attachment 195306 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195306&action=review > LayoutTests/editing/selection/click-on-anonymous-content-crash.html:1 > +<html> Missing DOCTYPE. > LayoutTests/editing/selection/click-on-anonymous-content-crash.html:3 > +<script src="../../fast/js/resources/js-test-pre.js"></script> Why do we need js-test-pre.js? > LayoutTests/editing/selection/click-on-anonymous-content-crash.html:9 > + if (window.internals) { Please check window.eventSender instead. internals is never used in this test.

Build Bot

Comment 13 2013-03-27 10:54:58 PDT

Comment on attachment 195278 [details] Patch Attachment 195278 [details] did not pass win-ews (win): Output: http://webkit-commit-queue.appspot.com/results/17323307

Build Bot

Comment 14 2013-03-27 22:07:47 PDT

Comment on attachment 195278 [details] Patch Attachment 195278 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17323354 New failing tests: editing/deleting/4922367.html

Build Bot

Comment 15 2013-03-27 22:07:50 PDT

Created attachment 195480 [details] Archive of layout-test-results from webkit-ews-04 for mac-future The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-future Platform: Mac OS X 10.8.2

Ryosuke Niwa

Comment 16 2013-03-27 22:13:56 PDT

It appears that we need to rebaseline this test: --- /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/editing/deleting/4922367-expected.txt +++ /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/editing/deleting/4922367-actual.txt @@ -9,17 +9,14 @@ text run at (386,0) width 390: "You should see only a table in the editable region below, with" text run at (0,18) width 139: "the caret just before it." RenderBlock {DIV} at (0,52) size 784x28 - RenderBlock {DIV} at (0,0) size 784x28 - RenderTable {TABLE} at (0,0) size 32x28 [border: (1px solid #BBBBBB)] - RenderTableSection {TBODY} at (1,1) size 30x26 - RenderTableRow {TR} at (0,2) size 30x22 - RenderTableCell {TD} at (2,2) size 12x22 [border: (1px solid #BBBBBB)] [r=0 c=0 rs=1 cs=1] - RenderText {#text} at (2,2) size 8x18 - text run at (2,2) width 8: "1" - RenderTableCell {TD} at (16,2) size 12x22 [border: (1px solid #BBBBBB)] [r=0 c=1 rs=1 cs=1] - RenderText {#text} at (2,2) size 8x18 - text run at (2,2) width 8: "2" - RenderBlock (anonymous) at (0,28) size 784x0 - RenderBlock (anonymous) at (0,28) size 784x0 + RenderTable {TABLE} at (0,0) size 32x28 [border: (1px solid #BBBBBB)] + RenderTableSection {TBODY} at (1,1) size 30x26 + RenderTableRow {TR} at (0,2) size 30x22 + RenderTableCell {TD} at (2,2) size 12x22 [border: (1px solid #BBBBBB)] [r=0 c=0 rs=1 cs=1] + RenderText {#text} at (2,2) size 8x18 + text run at (2,2) width 8: "1" + RenderTableCell {TD} at (16,2) size 12x22 [border: (1px solid #BBBBBB)] [r=0 c=1 rs=1 cs=1] + RenderText {#text} at (2,2) size 8x18 + text run at (2,2) width 8: "2" RenderBlock {UL} at (0,96) size 784x0 -caret: position 0 of child 0 {TABLE} of child 1 {DIV} of child 2 {DIV} of body +caret: position 0 of child 1 {TABLE} of child 2 {DIV} of body

Arpita Bahuguna

Comment 17 2013-03-28 01:17:01 PDT

Created attachment 195497 [details] Patch

Ryosuke Niwa

Comment 18 2013-03-28 01:19:39 PDT

?? Please rebaseline editing/deleting/4922367.html as needed.

Arpita Bahuguna

Comment 19 2013-03-28 01:24:15 PDT

(In reply to comment #18) > ?? Please rebaseline editing/deleting/4922367.html as needed. Hi rniwa, this was due to the previous (incorrect) patch I had uploaded : attachment number 195278. Not for the patch reviewed by you, i.e. #195306

Ryosuke Niwa

Comment 20 2013-03-28 02:01:45 PDT

Comment on attachment 195497 [details] Patch Okay.

Arpita Bahuguna

Comment 21 2013-03-28 02:07:42 PDT

(In reply to comment #20) > (From update of attachment 195497 [details]) > Okay. Thanks for the r+ ! :)

WebKit Review Bot

Comment 22 2013-03-28 02:11:37 PDT

Comment on attachment 195497 [details] Patch Clearing flags on attachment: 195497 Committed r147087: <http://trac.webkit.org/changeset/147087>

WebKit Review Bot

Comment 23 2013-03-28 02:11:42 PDT

All reviewed patches have been landed. Closing bug.

Yair Yogev

Comment 24 2013-03-28 17:35:27 PDT

Confirmed as fixed with Chromium (build 191215), Thanks.

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