Bug 128949 - setSelectionRange should set selection without validation
Summary: setSelectionRange should set selection without validation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 127832
  Show dependency treegraph
 
Reported: 2014-02-17 19:29 PST by Ryosuke Niwa
Modified: 2014-02-18 14:19 PST (History)
8 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
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>