autofocus processing model has been updated in https://github.com/whatwg/html/pull/4763 and https://github.com/whatwg/html/pull/5015 Implement that in WebKit.
<rdar://problem/56397019>
*** Bug 129743 has been marked as a duplicate of this bug. ***
*** Bug 82778 has been marked as a duplicate of this bug. ***
*** Bug 213982 has been marked as a duplicate of this bug. ***
*** Bug 202651 has been marked as a duplicate of this bug. ***
*** Bug 200913 has been marked as a duplicate of this bug. ***
Created attachment 438089 [details] WIP Here's some work-in-progress patch.
Created attachment 439376 [details] Patch
Rebased patch for EWS.
Relevant links: https://html.spec.whatwg.org/multipage/interaction.html#the-autofocus-attribute https://html.spec.whatwg.org/multipage/interaction.html#focusing-steps
Created attachment 440195 [details] Patch
Created attachment 440204 [details] Patch
Created attachment 440211 [details] Patch
Created attachment 440213 [details] Patch
Comment on attachment 440213 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440213&action=review > Source/WebCore/ChangeLog:9 > + Make WebKit match the new autofocus spec: You should summarize how the old behavior differs from the new behavior here, and what code changes were necessary to implement the new behavior.
Created attachment 440437 [details] Patch
Created attachment 440485 [details] Patch
Comment on attachment 440485 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440485&action=review Seems reasonable to me, but it would be good if Simon or Chris could take a look as well. (Also, do we know why form-submission-crash-3.html is timing out on the Windows bot?) > Source/WebCore/dom/Document.cpp:4481 > + m_autofocusCandidates.appendOrMoveToLast(candidate); Should we clear this out elsewhere as well? (e.g. `destroyRenderTree()` or `commonTeardown()`?) > Source/WebCore/dom/Document.cpp:4491 > + for (; !m_autofocusCandidates.isEmpty(); m_autofocusCandidates.removeFirst()) { > + Ref element = m_autofocusCandidates.first(); Nit - I think this might be a bit cleaner with a `while (!m_autofocusCandidates.isEmpty())` loop and `takeFirst()`.
(In reply to Wenson Hsieh from comment #18) > Comment on attachment 440485 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=440485&action=review > > Seems reasonable to me, but it would be good if Simon or Chris could take a > look as well. > > (Also, do we know why form-submission-crash-3.html is timing out on the > Windows bot?) Another race between the first rendering update and the onload event :) > > Source/WebCore/dom/Document.cpp:4481 > > + m_autofocusCandidates.appendOrMoveToLast(candidate); > > Should we clear this out elsewhere as well? (e.g. `destroyRenderTree()` or > `commonTeardown()`?) I guess we could clear it in commonTeardown().
Created attachment 440642 [details] Patch
Created attachment 440644 [details] Patch
Created attachment 440795 [details] Patch
Comment on attachment 440795 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440795&action=review > Source/WebCore/dom/Document.h:1809 > + ListHashSet<Ref<Element>> m_autofocusCandidates; Does this really need to hold a strong ref to the elements? Seems like it shouldn't and it would be less leak-prone to hold WeakPtrs. > Source/WebCore/dom/Document.h:1810 > + bool m_isAutofocusProcessed { false }; Please move this bool next to some other boolean data members for better packing.
Created attachment 440812 [details] Patch
Committed r283935 (242794@main): <https://commits.webkit.org/242794@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 440812 [details].