Reported downstream: http://code.google.com/p/chromium/issues/detail?id=86302 Reported by project member glider, Jun 16 (5 days ago) Chrome Version : 12.0.742.91 and 12.0.742.105 for Windows URLs (if applicable) : http://bpilot.xmlcan.ca/test/test_onkeyup.html Other browsers tested: Firefox 4.0.1 on Mac: OK Chrome 12.0.742.100 beta on Linux -- OK Chrome 11.0.696.71 on Mac -- OK What steps will reproduce the problem? 1. Go to http://bpilot.xmlcan.ca/test/test_onkeyup.html or open the following file locally: ============================================ <html> ....<input id='blabla' value='blabla' size='10' onkeyup='this.value=this.value.replace("d","")' onchange='alert(this.value)'/> </html> ============================================ 2. Type something into the input box 3. Click somewhere outside the input box What is the expected result? An alert with the input box contents shall fire What happens instead? Nothing
Created attachment 98057 [details] Patch
Comment on attachment 98057 [details] Patch Rejecting attachment 98057 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 2 Last 500 characters of output: ents-text.svg = TEXT Failed to run "['xvfb-run', 'Tools/Scripts/new-run-webkit-tests', '--chromium', '--no-pixel-tests', '--results-d..." exit_code: 2 sociated-element-crash.html = TEXT PASS fast/forms/input-number-events.html = TEXT PASS fast/forms/input-number-large-padding.html = TEXT PASS fast/forms/input-spinbutton-capturing.html = TEXT PASS Regressions: Unexpected text diff mismatch : (2) fast/forms/range-set-attribute.html = TEXT svg/custom/pointer-events-text.svg = TEXT Full output: http://queues.webkit.org/results/8925050
Created attachment 98066 [details] Archive of layout-test-results from ec2-cq-01 The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: ec2-cq-01 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment on attachment 98057 [details] Patch Attachment 98057 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8923080 New failing tests: fast/forms/range-set-attribute.html
Created attachment 98071 [details] Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Created attachment 98086 [details] Patch
PTAL
Comment on attachment 98086 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98086&action=review > Source/WebCore/ChangeLog:15 > + * html/HTMLInputElement.cpp: > + (WebCore::HTMLInputElement::setValue): > + * html/HTMLTextAreaElement.cpp: > + (WebCore::HTMLTextAreaElement::setValue): > + (WebCore::HTMLTextAreaElement::setNonDirtyValue): > + (WebCore::HTMLTextAreaElement::setValueCommon): Could you add something to the change log that explains why these changes are correct? > Source/WebCore/html/HTMLInputElement.cpp:982 > + String sanitizedValue = this->sanitizeValue(value); Why the this-> on this line?
> Could you add something to the change log that explains why these changes are correct? Will do. > > > Source/WebCore/html/HTMLInputElement.cpp:982 > > + String sanitizedValue = this->sanitizeValue(value); > > Why the this-> on this line? To avoid confusion as sanitizeValue looks a lot like sanitizedValue. Perhaps I should try to pick a better name for the local variable.
The regression test has a disconnect from bug title. Does this only change the behavior for keyup? What if the value is set from keydown or keypress, and then default handling is prevented?
keypress fires before the value is updated and can't be prevented so its behavior is unaffected by this change. I've added tests for keydown though. Thanks Alexey!
Created attachment 98233 [details] Patch for landing
(In reply to comment #11) > keypress fires before the value is updated and can't be prevented so its behavior is unaffected by this change. I don't follow what you are saying. Keydown is also fired before the value is updated, so what's the difference? And no, preventing default handling of keypress does work.
Created attachment 98240 [details] Patch You are right, keypress can be canceled. Updated the test case accordingly.
Alexey, is there anything else you'd like me to test or are you happy with the test for this patch now that I test keydown, keypress and keyup? Thanks.
I don't have any other comment, thank you for adding the tests. Since the patch already has reviewers filled in, you can remove r? flag, and set cq? or cq+.
Comment on attachment 98240 [details] Patch Clearing flags on attachment: 98240 Committed r89624: <http://trac.webkit.org/changeset/89624>
All reviewed patches have been landed. Closing bug.