Bug 112992

Created attachment 194418 [details] Patch

Comment on attachment 194418 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194418&action=review > Source/WebCore/dom/Document.cpp:1904 > + Should we do this here or do the check on Document::activeElement()? My concerns are: - Whether this is a "catch all" location. - If we want to restore focus after setting display property back to something other than "none".

Comment on attachment 194418 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194418&action=review >> Source/WebCore/dom/Document.cpp:1904 >> + > > Should we do this here or do the check on Document::activeElement()? > My concerns are: > - Whether this is a "catch all" location. > - If we want to restore focus after setting display property back to something other than "none". > - If we want to restore focus after setting display property back to something other than "none". good point. I confirmed IE, Firefox, and Opera kept focus state when we set display:none then display:block. This patch is not compatible.

Comment on attachment 194418 [details] Patch Attachment 194418 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17193608 New failing tests: fast/events/tab-imagemap.html accessibility/canvas-fallback-content-2.html fast/events/mouse-focus-imagemap.html fast/events/imagemap-norender-crash.html accessibility/canvas-fallback-content.html platform/chromium/virtual/gpu/fast/canvas/fallback-content.html fast/events/tab-focus-link-in-canvas.html fast/canvas/fallback-content.html

Created attachment 194438 [details] Archive of layout-test-results from gce-cr-linux-08 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-08 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04

(In reply to comment #4) > New failing tests: > fast/events/tab-imagemap.html > accessibility/canvas-fallback-content-2.html > fast/events/mouse-focus-imagemap.html > fast/events/imagemap-norender-crash.html > accessibility/canvas-fallback-content.html > platform/chromium/virtual/gpu/fast/canvas/fallback-content.html > fast/events/tab-focus-link-in-canvas.html > fast/canvas/fallback-content.html Hmm, we must check isFocusable. Some elements are focusable even if they don't have renderers.

Created attachment 194447 [details] WIP

> This bug is serious because implicit form submission by Enter key can proceed with invisible text inputs. Does it work in other browsers? Sometimes sites focus an invisible element intentionally, so that they could more deeply customize the UI. http://tryruby.org used to employ this trick to draw a custom caret, and judging from the looks, it might still be. I don't remember if it was display:none or visibility:hidden, however the rationale you posted applies to both equally.

(In reply to comment #8) > > This bug is serious because implicit form submission by Enter key can proceed with invisible text inputs. > > Does it work in other browsers? IE and Firefox don't have this bug, and the HTML standard clearly says we should loose focus when a focused element becomes non-focusable.

We should resolve Bug 32292 before this. Plugins would loose focus if we landed a patch like one I posted.

(In reply to comment #3) > > - If we want to restore focus after setting display property back to something other than "none". > > good point. I confirmed IE, Firefox, and Opera kept focus state when we set display:none then display:block. This patch is not compatible. Ah, my test code was wrong! They don't restore focus state when the element becomes focusable again. We don't need to record the last focused node.

We fixed this in Blink by http://src.chromium.org/viewvc/blink?view=revision&revision=150870 . However we feel the fix is not cool and wondering if there is a better way.

*** Bug 116925 has been marked as a duplicate of this bug. ***

*** This bug has been marked as a duplicate of bug 29241 ***