Bug 128949

Summary:

setSelectionRange should set selection without validation

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

Forms

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, benjamin, buildbot, darin, enrica, kling, koivisto, rniwa

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

127832

Attachments:

Description	Flags
Fixes the bug	none
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2	none
Removed ML specific expected results	none
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2	none
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion	none
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion	none
Fix ML for the last time	enrica: review+

Ryosuke Niwa

Reported 2014-02-17 19:29:18 PST

Since all text nodes and br elements are visible inside text form control elements, we don't have to validate manually to set selection once we call positionForIndex in setSelectionRange. This would avoid triggering synchronous layout.

Attachments
Fixes the bug (20.62 KB, patch) 2014-02-17 19:59 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (471.96 KB, application/zip) 2014-02-17 20:52 PST, Build Bot	no flags	Details
Removed ML specific expected results (28.46 KB, patch) 2014-02-17 20:59 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 (472.40 KB, application/zip) 2014-02-17 22:00 PST, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (502.07 KB, application/zip) 2014-02-17 22:31 PST, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion (504.44 KB, application/zip) 2014-02-17 23:36 PST, Build Bot	no flags	Details
Fix ML for the last time (25.09 KB, patch) 2014-02-18 11:36 PST, Ryosuke Niwa	enrica: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Ryosuke Niwa

Comment 1 2014-02-17 19:59:51 PST

Created attachment 224460 [details] Fixes the bug

Build Bot

Comment 2 2014-02-17 20:52:26 PST

Comment on attachment 224460 [details] Fixes the bug Attachment 224460 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6464277095383040 New failing tests: platform/mac/editing/spelling/autocorrection-at-beginning-of-word-2.html platform/mac/editing/spelling/autocorrection-at-beginning-of-word-1.html

Build Bot

Comment 3 2014-02-17 20:52:27 PST

Created attachment 224464 [details] Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-14 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5

Ryosuke Niwa

Comment 4 2014-02-17 20:59:43 PST

Created attachment 224465 [details] Removed ML specific expected results

Build Bot

Comment 5 2014-02-17 22:00:43 PST

Comment on attachment 224465 [details] Removed ML specific expected results Attachment 224465 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6209633853112320 New failing tests: platform/mac/editing/spelling/autocorrection-at-beginning-of-word-2.html platform/mac/editing/spelling/autocorrection-at-beginning-of-word-1.html

Build Bot

Comment 6 2014-02-17 22:00:45 PST

Created attachment 224469 [details] Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5

Build Bot

Comment 7 2014-02-17 22:31:15 PST

Comment on attachment 224465 [details] Removed ML specific expected results Attachment 224465 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5680356980162560 New failing tests: platform/mac/editing/spelling/autocorrection-at-beginning-of-word-2.html platform/mac/editing/spelling/autocorrection-at-beginning-of-word-1.html

Build Bot

Comment 8 2014-02-17 22:31:17 PST

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

Build Bot

Comment 9 2014-02-17 23:36:56 PST

Comment on attachment 224465 [details] Removed ML specific expected results Attachment 224465 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4696077714325504 New failing tests: platform/mac/editing/spelling/autocorrection-at-beginning-of-word-2.html platform/mac/editing/spelling/autocorrection-at-beginning-of-word-1.html

Build Bot

Comment 10 2014-02-17 23:36:59 PST

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

Ryosuke Niwa

Comment 11 2014-02-18 11:36:13 PST

Created attachment 224528 [details] Fix ML for the last time

Enrica Casucci

Comment 12 2014-02-18 13:50:53 PST

Comment on attachment 224528 [details] Fix ML for the last time View in context: https://bugs.webkit.org/attachment.cgi?id=224528&action=review Looks ok to me. > Source/WebCore/dom/Position.h:-279 > - return Position(Position::findParent(node), node->nodeIndex(), Position::PositionIsOffsetInAnchor); Should you ASSERT node->parentNode() here too?

Ryosuke Niwa

Comment 13 2014-02-18 13:52:03 PST

Comment on attachment 224528 [details] Fix ML for the last time View in context: https://bugs.webkit.org/attachment.cgi?id=224528&action=review >> Source/WebCore/dom/Position.h:-279 >> - return Position(Position::findParent(node), node->nodeIndex(), Position::PositionIsOffsetInAnchor); > > Should you ASSERT node->parentNode() here too? The comment above it says why we can't assert it here but I'll check if it's still the case.

Ryosuke Niwa

Comment 14 2014-02-18 14:19:33 PST

Committed r164316: <http://trac.webkit.org/changeset/164316>

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