Bug 128949

Summary: setSelectionRange should set selection without validation
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: FormsAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, buildbot, darin, enrica, kling, koivisto, rniwa
Priority: P2    
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+

Description Ryosuke Niwa 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.
Comment 1 Ryosuke Niwa 2014-02-17 19:59:51 PST
Created attachment 224460 [details]
Fixes the bug
Comment 2 Build Bot 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
Comment 3 Build Bot 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
Comment 4 Ryosuke Niwa 2014-02-17 20:59:43 PST
Created attachment 224465 [details]
Removed ML specific expected results
Comment 5 Build Bot 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
Comment 6 Build Bot 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
Comment 7 Build Bot 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
Comment 8 Build Bot 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
Comment 9 Build Bot 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
Comment 10 Build Bot 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
Comment 11 Ryosuke Niwa 2014-02-18 11:36:13 PST
Created attachment 224528 [details]
Fix ML for the last time
Comment 12 Enrica Casucci 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?
Comment 13 Ryosuke Niwa 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.
Comment 14 Ryosuke Niwa 2014-02-18 14:19:33 PST
Committed r164316: <http://trac.webkit.org/changeset/164316>