Bug 145430

Summary:

When the value of input isnot changed , selection range shouldnot be changed

Product:

WebKit

Reporter:

Peng Xinchao <xinchao.peng>

Component:

HTML Editing

Assignee:

Nobody <webkit-unassigned>

Status:

NEW

Severity:

Normal

CC:

beidson, buildbot, commit-queue, d_russell, jdiggs, rniwa, zalan

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	rniwa: review-, buildbot: commit-queue-
Archive of layout-test-results from ews104 for mac-mavericks-wk2	none
Patch	none
Patch	beidson: review-

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	Details Formatted Diff Diff
Patch (1.71 KB, patch) 2015-05-28 18:57 PDT, Peng Xinchao	no flags	Details Formatted Diff Diff
Patch (1.45 KB, patch) 2015-05-31 18:29 PDT, Peng Xinchao	rniwa: review- buildbot: commit-queue-	Details Formatted Diff Diff
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	Details Formatted Diff Diff
Patch (4.28 KB, patch) 2015-06-01 23:16 PDT, Peng Xinchao	beidson: review-	Details Formatted Diff Diff
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.