NEW 145430
When the value of input isnot changed , selection range shouldnot be changed 
https://bugs.webkit.org/show_bug.cgi?id=145430
Summary When the value of input isnot changed , selection range shouldnot be changed
Peng Xinchao
Reported 2015-05-27 22:58:45 PDT
Test Case: <html> <head> <title>Jasmine Spec Runner</title> <script> function testchangeime() { var q = document.getElementById("username").value; document.getElementById("username").setSelectionRange(0,q.length); var p = q.trim(); document.getElementById("username").value = p; } </script> </head> <body> <input type="text" id="username" oninput="testchangeime()"> </input> </body> </html> When user input "a" "b" "c" "d" continuously，the value of input is "abcd". But i think the value of input should be "d".
Attachments
Patch (1.74 KB, patch)
2015-05-28 18:51 PDT, Peng Xinchao 		no flags
DetailsFormatted DiffDiff
Patch (1.71 KB, patch)
2015-05-28 18:57 PDT, Peng Xinchao 		no flags
DetailsFormatted DiffDiff
Patch (1.45 KB, patch)
2015-05-31 18:29 PDT, Peng Xinchao 		rniwa: review-
buildbot: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (642.77 KB, application/zip)
2015-05-31 19:08 PDT, Build Bot 		no flags
Details
Patch (2.25 KB, patch)
2015-06-01 22:48 PDT, Peng Xinchao 		no flags
DetailsFormatted DiffDiff
Patch (4.28 KB, patch)
2015-06-01 23:16 PDT, Peng Xinchao 		beidson: review-
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
zalan
Comment 1 2015-05-28 08:48:34 PDT
Apparently the "document.getElementById("username").value" removes the selection so the next input entry gets appended (instead of replacing the selected content). Chrome behaves like Safari, while Firefox behaves differently (as described as expected behavior)
Peng Xinchao
Comment 2 2015-05-28 18:51:46 PDT
Created attachment 253887 [details] Patch
WebKit Commit Bot
Comment 3 2015-05-28 18:53:26 PDT
Attachment 253887 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Peng Xinchao
Comment 4 2015-05-28 18:57:01 PDT
Created attachment 253888 [details] Patch
Ryosuke Niwa
Comment 5 2015-05-28 21:59:26 PDT
Comment on attachment 253888 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253888&action=review r- because this patch is missing a test. I'm also worried that this change is not backwards compatible and may worsen the certain aspects of user interaction. > Source/WebCore/html/TextFieldInputType.cpp:129 > + if (Frame* frame = input->document()->frame()) { > + start = frame->selection()->selection().start().offsetInContainerNode(); > + end = frame->selection()->selection().end().offsetInContainerNode(); > + } This is wrong. containerNode could be a div in the shadow DOM. We should be using element().selectionStart() and element().selectionEnd() instead.
Peng Xinchao
Comment 6 2015-05-31 18:29:23 PDT
Created attachment 253985 [details] Patch
Build Bot
Comment 7 2015-05-31 19:08:13 PDT
Comment on attachment 253985 [details] Patch Attachment 253985 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6746635820335104 New failing tests: fast/frames/take-focus-from-iframe.html platform/mac/editing/input/selection-change-closes-typing.html fast/forms/input-setvalue-selection.html
Build Bot
Comment 8 2015-05-31 19:08:17 PDT
Created attachment 253986 [details] Archive of layout-test-results from ews104 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Ryosuke Niwa
Comment 9 2015-05-31 19:10:30 PDT
Comment on attachment 253985 [details] Patch Again, this patch definitely needs a layout test, and we need to test backwards compatibility of this change.
Peng Xinchao
Comment 10 2015-06-01 22:48:25 PDT
Created attachment 254052 [details] Patch
Peng Xinchao
Comment 11 2015-06-01 23:16:51 PDT
Created attachment 254056 [details] Patch
Darin Adler
Comment 12 2015-06-02 11:43:25 PDT
Comment on attachment 254056 [details] Patch The old behavior was intentional, originally to match what other web browsers do. What’s the web compatibility impact of changing this?
Peng Xinchao
Comment 13 2015-06-02 18:26:31 PDT
It happened with IME of PreEdit . Example : <input type="text" id="username" oninput="testchangeime()"> </input> <script> function testchangeime(){ var q = document.getElementById("username").value; var p = q.trim(); document.getElementById("username").value = p; } </script> When user input 'a' with IME ,the status of 'a' is PreEdit . But if testchangeime() is called , the status of 'a' will become PreEdit to NoPreEdit.
Brady Eidson
Comment 14 2017-04-24 19:11:55 PDT
Comment on attachment 254056 [details] Patch This patch has been pending review since 2015 with no recent activity. It seems unlikely that it would even still apply to trunk in its current form. Clearing from the review queue. Feel free to update and resubmit if the patch is still relevant.
Note You need to log in before you can comment on or make changes to this bug.