Imported from http://crbug.com/158802 Focused select element is changeable even if it is disabled. Example URL: http://jsfiddle.net/langdonx/ymEZR/ If focused element becomes disabled, the element should lose focus. This is expected behavior within HTML5 spec: http://www.whatwg.org/specs/web-apps/current-work/multipage/editing.html#focus-management
Created attachment 171801 [details] Patch
Comment on attachment 171801 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171801&action=review > Source/WebCore/html/HTMLFormControlElement.cpp:154 > + if (disabled() && focused()) > + blur(); > setNeedsWillValidateCheck(); Unfortunately, this is not acceptable. blur() dispatches a 'blur' event, and Event handler JavaScript code can delete |this| object. Probably we should make blur/focusout/DOMFocusOut/focus/focusin/DOMFocusIn events scoped-events, like I did in http://trac.webkit.org/changeset/132983
When fixing this, please also add a test case for the issue Kent found in review.
Created attachment 172905 [details] Patch 2
Comment on attachment 172905 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=172905&action=review > Source/WebCore/ChangeLog:15 > + (WebCore::HTMLSelectElement::menuListDefaultEventHandler): > + (WebCore::HTMLSelectElement::listBoxDefaultEventHandler): > + (WebCore::HTMLSelectElement::defaultEventHandler): You changes are correct, but I think checking disabled() in the beginning of HTMLSelectElement::defaultEventHandler is enough and simpler.
Created attachment 172911 [details] Patch 3
Comment on attachment 172905 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=172905&action=review >> Source/WebCore/ChangeLog:15 >> + (WebCore::HTMLSelectElement::defaultEventHandler): > > You changes are correct, but I think checking disabled() in the beginning of HTMLSelectElement::defaultEventHandler is enough and simpler. Done, thanks.
Comment on attachment 172911 [details] Patch 3 View in context: https://bugs.webkit.org/attachment.cgi?id=172911&action=review > Source/WebCore/html/HTMLSelectElement.cpp:1443 > + if (disabled() && (event->type() == eventNames().keydownEvent || event->type() == eventNames().keypressEvent)) { if (disabled()) { should be enough.
Created attachment 172918 [details] Patch 4
Comment on attachment 172911 [details] Patch 3 View in context: https://bugs.webkit.org/attachment.cgi?id=172911&action=review >> Source/WebCore/html/HTMLSelectElement.cpp:1443 >> + if (disabled() && (event->type() == eventNames().keydownEvent || event->type() == eventNames().keypressEvent)) { > > if (disabled()) { > should be enough. Done.
Comment on attachment 172918 [details] Patch 4 ok
Comment on attachment 172918 [details] Patch 4 Rejecting attachment 172918 [details] from commit-queue. New failing tests: inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html Full output: http://queues.webkit.org/results/14766341
Comment on attachment 172918 [details] Patch 4 Clearing flags on attachment: 172918 Committed r133860: <http://trac.webkit.org/changeset/133860>
All reviewed patches have been landed. Closing bug.