Add focusing changes + monkeypatches #68
Conversation
|
One question: Do we need to modify https://html.spec.whatwg.org/multipage/interaction.html#sequential-navigation-search-algorithm? I guess we'll catch this with the audit anyways, in which case we don't have to modify it in this PR, right? |
spec.bs
Outdated
| <ol> | ||
| <li><p>Set <var ignore=''>element</var>'s [=previously focused element=] to null.</p></li> | ||
|
|
||
| <li><p>If <var ignore=''>focusPreviousElement</var> is true, then run the [=focusing steps=] for |
There was a problem hiding this comment.
This step is identical to the one in the spec, and so is the above. Does this patch actually do anything?
There was a problem hiding this comment.
I forgot to add the unfenced part to this, good catch.
spec.bs
Outdated
| container/fenced navigable=], and |old chain| does not also [=list/contain=] |element|, then | ||
| return. | ||
|
|
||
| Note: This is how we bail-out early just before calling the [=focus update steps=], in the case |
There was a problem hiding this comment.
I added this Note:. Does it capture the intent correctly? If so (that is, if I understand what our aim is here), then I think the above step is not adequate. If you move focus from one element inside of a fenced frame to another element inside of a fenced frame, I think the logic in your step will trigger and then we'll mistakenly early-return, right? Does that logic make sense?
There was a problem hiding this comment.
I think it should be checking for a document instead of an element. That should correctly handle the case of switching focus between 2 elements in a fenced frame.
Both focus chains will contain the fenced frame root's document at that point, and the function will not early return. If we switch focus from one fenced frame to another, now both of the fenced frame root documents will only exist in one of the focus chains, and the function will early return.
add rest of cases in audit
add rest of focus algorithms
fix build
Some format nits
|
I added some formatting and editorial improvements, I'll try and fix the conflicts since I caused them. |
939b0fc to
a06b792
Compare
…rame into liam-focusing-steps
I think we might need to modify it. This is operating under the assumption that "If candidate is a navigable container with a non-null content navigable" will not return anything for a fenced frame, so we'd need to add an extra case to do the same thing but with fenced navigables. I am assuming that "starting point is a navigable" will be okay though, since a fenced navigable is still a navigable. We could punt on this until we get to the "content navigable" stuff, but there's a ton of entries in that category, so it might be simpler to just do this now. |
| [=fenced navigable container/fenced navigable=], whichever is non-null. | ||
| </div> | ||
|
|
||
| <div algorithm=sequential-focus-navigation-patch> |
There was a problem hiding this comment.
All of the sequential focus and navigation changes are unfenced by default it seems. I just want to double check: that's because there are only initiated with user actions, right? I think I'm going to land this right not but please respond here after just for posterity
There was a problem hiding this comment.
SHA: fa7833d Reason: push, by domfarolino Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Preview | Diff