Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update "iframe with interactive elements is not excluded from tab-order" rule to manage inert iframe elements #2038

Conversation

giacomo-petri
Copy link
Collaborator

@giacomo-petri giacomo-petri commented Feb 23, 2023

This pull request manages inert iframe elements in iframe with interactive elements is not excluded from tab-order rule, excluding them from the applicability of the rule.

Updates:

  • edited applicability excluding inert iframe elements;
  • added definition of inert in applicability;
  • added definition of modal (modal dialog);
  • added 2 inapplicable examples:
    • iframe with inert attribute
    • modal which overlaps the iframe, making the iframe inoperable

Closes issue(s): #1965

Need for Call for Review:
This will require a 2 weeks Call for Review


Pull Request Etiquette

When creating PR:

  • Make sure you're requesting to pull a branch (right side) to the develop branch (left side).
  • Make sure you do not remove the "How to Review and Approve" section in your pull request description

After creating PR:

  • Add yourself (and co-authors) as "Assignees" for PR.
  • Add label to indicate if it's a Rule, Definition or Chore.
  • Link the PR to any issue it solves. This will be done automatically by referencing the issue at the top of this comment in the indicated place.
  • Optionally request feedback from anyone in particular by assigning them as "Reviewers".

When merging a PR:

  • Close any issue that the PR resolves. This will happen automatically upon merging if the PR was correctly linked to the issue, e.g. by referencing the issue at the top of this comment.

How to Review And Approve

  • Go to the “Files changed” tab
  • Here you will have the option to leave comments on different lines.
  • Once the review is completed, find the “Review changes” button in the top right, select “Approve” (if you are really confident in the rule) or "Request changes" and click “Submit review”.
  • Make sure to also review the proposed Call for Review period. In case of disagreement, the longer period wins.

@giacomo-petri giacomo-petri self-assigned this Feb 23, 2023

- the element is [visible][]; and
- the element is part of the [sequential focus navigation order][] of the `iframe`'s [document][].

An element is <dfn id="akn7bn:contain">contained</dfn> in a [nested browsing context][] if its [owner document][] is the [container document][] of the [nested browsing context][].

An `iframe` element is <dfn id="akn7bn:inert">inert</dfn> if:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can have this as a definition also, to make it reusable in other (future?) rules

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And probably it would be better to have a definite of inert element, not just inert iframe

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, we should have a separate definition of "inert element".


- the element is [visible][]; and
- the element is part of the [sequential focus navigation order][] of the `iframe`'s [document][].

An element is <dfn id="akn7bn:contain">contained</dfn> in a [nested browsing context][] if its [owner document][] is the [container document][] of the [nested browsing context][].

An `iframe` element is <dfn id="akn7bn:inert">inert</dfn> if:
- it has an `inert` [attribute value][] of true; or
- an element which is not the iframe itself and that is not [contained](#akn7bn:contain) in it behaves as a [modal][], making the iframe inoperable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "behaves as a modal" mean?
And do we want to tie this to the behaviour or the state? If there is a modal open, the iframe is inert and not applicable, but if the modal is closed, then the iframe is applicable. In both states, the component behaves as a modal, right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can probably refer directly to the HTML definition of inert.
It doesn't refer explicitly to modals, but being blocked by a modal causes most of the document to be inert, so we can mention that.

If we want to have our own definition of inert, we should probably refer to "being blocked by a modal", since it is objective and unambiguous, and a common definition from higher spec.
This may leave room for custom modals where somebody uses a div and a ton of Javascript to turn it into a modal manually. We might be OK enough by having an assumption about people not being that crazy… (i.e. I do not know how frequent it is for devs to re-code modal behaviour on their own rather than relying on native semantics…)

- DOM tree
---

A modal dialog is a type of user interface element that appears on top of the main content of a [html web page][] or application and requires the user to take some action or make a decision before they can continue interacting with the program.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think "type of user interface" is objective.
Same for "main content".
What is the "program" you mention?

I think this needs to focus on the element preventing interaction with all other elements that are not their children.

This definition really needs to be objective, because it is used in the applicability of the rule.


- the element is [visible][]; and
- the element is part of the [sequential focus navigation order][] of the `iframe`'s [document][].

An element is <dfn id="akn7bn:contain">contained</dfn> in a [nested browsing context][] if its [owner document][] is the [container document][] of the [nested browsing context][].

An `iframe` element is <dfn id="akn7bn:inert">inert</dfn> if:
- it has an `inert` [attribute value][] of true; or
- an element which is not the iframe itself and that is not [contained](#akn7bn:contain) in it behaves as a [modal][], making the iframe inoperable.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- an element which is not the iframe itself and that is not [contained](#akn7bn:contain) in it behaves as a [modal][], making the iframe inoperable.
- an element which is not the iframe itself and that is not [contained](#akn7bn:contain) in it makes the iframe and all of its contents [inoperable][]. (For example: a modal.)

Does that improve the situation at all?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant by modal dialogs was the actual <dialog> element with showModal() called on it. That programmatically makes all other content on the page inert. I think you could define it like this:

The element is in a page with a dialog element with the is-modal flag set to true, and is not a descendant of that dialog element.

I'm not sure we should try for a generic modal definition. I don't know how you'd do that in a way that's objective.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then, we are indeed going for blocked by a modal:

A Document document is blocked by a modal dialog subject if subject is the topmost dialog element in document's top layer. While document is so blocked, every node that is connected to document, with the exception of the subject element and its flat tree descendants, must become inert.

subject can additionally become inert via the inert attribute, but only if specified on subject itself (i.e., subject escapes inertness of ancestors); subject's flat tree descendants can become inert in a similar fashion.

Note: The dialog element's showModal() method causes this mechanism to trigger, by adding the dialog element to its node document's top layer.

</div>
<iframe tabindex="-1" srcdoc="<a href='/'>Home</a>"></iframe>
```

[attribute value]: #attribute-value 'Definition of Attribute Value'
[container document]: https://html.spec.whatwg.org/#bc-container-document 'HTML browsing context container document, 2020/12/18'
[document]: https://html.spec.whatwg.org/multipage/dom.html#document 'HTML definition of document'
[flattened tabindex-ordered focus navigation scope]: https://html.spec.whatwg.org/multipage/interaction.html#flattened-tabindex-ordered-focus-navigation-scope 'HTML - Living Standard, 2022/07/08'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[flattened tabindex-ordered focus navigation scope]: https://html.spec.whatwg.org/multipage/interaction.html#flattened-tabindex-ordered-focus-navigation-scope 'HTML - Living Standard, 2022/07/08'
[flattened tabindex-ordered focus navigation scope]: https://html.spec.whatwg.org/multipage/interaction.html#flattened-tabindex-ordered-focus-navigation-scope 'HTML - Living Standard, 2022/07/08'
[inoperable]: https://www.w3.org/TR/wai-aria/#dfn-operable

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks outdated?

pages/glossary/modal-dialog.md Outdated Show resolved Hide resolved

- the element is [visible][]; and
- the element is part of the [sequential focus navigation order][] of the `iframe`'s [document][].

An element is <dfn id="akn7bn:contain">contained</dfn> in a [nested browsing context][] if its [owner document][] is the [container document][] of the [nested browsing context][].

An `iframe` element is <dfn id="akn7bn:inert">inert</dfn> if:
- it has an `inert` [attribute value][] of true; or
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The attribute value doesn't matter. It just has to be present. Plus this should apply to ancestors too. Inert disables the entire subtree.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The attribute value doesn't matter. It just has to be present.

As per our definition of attribute value, boolean attribute (like inert) have an "attribute value" of true if the attribute is present.


- the element is [visible][]; and
- the element is part of the [sequential focus navigation order][] of the `iframe`'s [document][].

An element is <dfn id="akn7bn:contain">contained</dfn> in a [nested browsing context][] if its [owner document][] is the [container document][] of the [nested browsing context][].

An `iframe` element is <dfn id="akn7bn:inert">inert</dfn> if:
- it has an `inert` [attribute value][] of true; or
- an element which is not the iframe itself and that is not [contained](#akn7bn:contain) in it behaves as a [modal][], making the iframe inoperable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant by modal dialogs was the actual <dialog> element with showModal() called on it. That programmatically makes all other content on the page inert. I think you could define it like this:

The element is in a page with a dialog element with the is-modal flag set to true, and is not a descendant of that dialog element.

I'm not sure we should try for a generic modal definition. I don't know how you'd do that in a way that's objective.

@@ -0,0 +1,16 @@
---
title: Modal dialog
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned this could be confused by the <dialog>'s showModal() method.

@dan-tripp-siteimprove dan-tripp-siteimprove self-requested a review March 8, 2023 19:37

- the element is [visible][]; and
- the element is part of the [sequential focus navigation order][] of the `iframe`'s [document][].

An element is <dfn id="akn7bn:contain">contained</dfn> in a [nested browsing context][] if its [owner document][] is the [container document][] of the [nested browsing context][].

An `iframe` element is <dfn id="akn7bn:inert">inert</dfn> if:
- it has an `inert` [attribute value][] of true; or
- an element which is not the iframe itself and that is not [contained](#akn7bn:contain) in it behaves as a [modal][], making the iframe inoperable.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- an element which is not the iframe itself and that is not [contained](#akn7bn:contain) in it behaves as a [modal][], making the iframe inoperable.
- it is on the same page as a `dialog` element which has its [`is-modal`](https://html.spec.whatwg.org/multipage/interactive-elements.html#is-modal) flag set to `true`, and is not a descendant of that `dialog` element.

@Jym77 Jym77 dismissed stale reviews from WilcoFiers, carlosapaduarte, and dan-tripp-siteimprove April 27, 2023 08:17

Changes were done

Jym77
Jym77 previously requested changes Apr 27, 2023
Copy link
Collaborator

@Jym77 Jym77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To sum up, I think we can just use HTML definition of inert (instead of trying to rebuild it) + add some background description about showModal triggering "blocked by a modal" triggering inert + make the examples descriptions clear (this iframe is inert because it is blocked by the modal that has been brought to the top layer with showModal).


- the element is [visible][]; and
- the element is part of the [sequential focus navigation order][] of the `iframe`'s [document][].

An element is <dfn id="akn7bn:contain">contained</dfn> in a [nested browsing context][] if its [owner document][] is the [container document][] of the [nested browsing context][].

An `iframe` element is <dfn id="akn7bn:inert">inert</dfn> if:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, we should have a separate definition of "inert element".


- the element is [visible][]; and
- the element is part of the [sequential focus navigation order][] of the `iframe`'s [document][].

An element is <dfn id="akn7bn:contain">contained</dfn> in a [nested browsing context][] if its [owner document][] is the [container document][] of the [nested browsing context][].

An `iframe` element is <dfn id="akn7bn:inert">inert</dfn> if:
- it has an `inert` [attribute value][] of true; or
- an element which is not the iframe itself and that is not [contained](#akn7bn:contain) in it behaves as a [modal][], making the iframe inoperable.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can probably refer directly to the HTML definition of inert.
It doesn't refer explicitly to modals, but being blocked by a modal causes most of the document to be inert, so we can mention that.

If we want to have our own definition of inert, we should probably refer to "being blocked by a modal", since it is objective and unambiguous, and a common definition from higher spec.
This may leave room for custom modals where somebody uses a div and a ton of Javascript to turn it into a modal manually. We might be OK enough by having an assumption about people not being that crazy… (i.e. I do not know how frequent it is for devs to re-code modal behaviour on their own rather than relying on native semantics…)


- the element is [visible][]; and
- the element is part of the [sequential focus navigation order][] of the `iframe`'s [document][].

An element is <dfn id="akn7bn:contain">contained</dfn> in a [nested browsing context][] if its [owner document][] is the [container document][] of the [nested browsing context][].

An `iframe` element is <dfn id="akn7bn:inert">inert</dfn> if:
- it has an `inert` [attribute value][] of true; or
- an element which is not the iframe itself and that is not [contained](#akn7bn:contain) in it behaves as a [modal][], making the iframe inoperable.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then, we are indeed going for blocked by a modal:

A Document document is blocked by a modal dialog subject if subject is the topmost dialog element in document's top layer. While document is so blocked, every node that is connected to document, with the exception of the subject element and its flat tree descendants, must become inert.

subject can additionally become inert via the inert attribute, but only if specified on subject itself (i.e., subject escapes inertness of ancestors); subject's flat tree descendants can become inert in a similar fashion.

Note: The dialog element's showModal() method causes this mechanism to trigger, by adding the dialog element to its node document's top layer.

@giacomo-petri giacomo-petri requested a review from bruce-usab May 9, 2023 07:36
@Jym77
Copy link
Collaborator

Jym77 commented May 9, 2023

Title edited. The initial intent was to provide information about the specific update, avoiding a generic label such as "Update iframe rule", which doesn't help in filtering PR. Please, let me know if the new title makes sense.

As a rule of thumb, I feel it is good to include the full name of the updated rule in the PR title. Including the rule ID can also help tracking them across change of name (the id is the stable name). I usually like to frontload these, but we have no strict PR naming convention.

"rule name" [rule ID]: verb something something.
e.g.
"iframe with interactive elements is not excluded from tab-order" [akn7bn]: Manage inert iframe element

(I feel that "update rule" is a bit redundant in a PR title, since PR are mostly made to update rules)
(the current title is fine, no need to change it again)

giacomo-petri

This comment was marked as duplicate.

@giacomo-petri
Copy link
Collaborator Author

  • Accepted all the @Jym77's suggestions about adding the tabindex="-1" attribute to the iframe element;
  • Updated the Failed example n. 6 with dialog opened onload instead of after user activation;
  • Removed failed example n.7 with "custom modal" as not aligned with the rule;
  • Updated references in "inert" definition.

@giacomo-petri giacomo-petri dismissed Jym77’s stale review May 25, 2023 11:12

suggestions applied and highlighted items resolved

@giacomo-petri giacomo-petri requested a review from Jym77 May 25, 2023 11:12
@giacomo-petri giacomo-petri dismissed dan-tripp-siteimprove’s stale review May 25, 2023 11:13

we moved with the definition of blocked by a modal, that should solve the item highlighted

@carlosapaduarte
Copy link
Member

Call for review ends June 21

Copy link
Collaborator

@bruce-usab bruce-usab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

From the list email, I had expected I would have concern about defining inert. Instead, I was delighted to learn that it is a reasonably well defined attribute.

I don't think my own knowledge is too below average, so I recommend adding links for background reading. Here are two I found useful:

@Jym77
Copy link
Collaborator

Jym77 commented Jun 8, 2023

I don't think my own knowledge is too below average, so I recommend adding links for background reading. Here are two I found useful:

@bruce-usab Since ACT rules are intended to be published as official W3C documentation, we tend to avoid direct links (or citation) of commercial third parties, especially when they are themselves major players (we do not want to look like we favour one above the others). As much as we can, we'd rather stick with W3C documentation (HTML living standard, …)

Copy link
Member

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple minor points. Don't expect any discussion necessary on these.

pages/glossary/inert.md Outdated Show resolved Hide resolved
pages/glossary/inert.md Show resolved Hide resolved
@giacomo-petri
Copy link
Collaborator Author

This PR is in call for review for 2 weeks. Review ends Sept 29th.

@giacomo-petri giacomo-petri added the Review call 2 weeks Call for review for new rules and big changes label Sep 15, 2023
Copy link
Member

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review call 2 weeks Call for review for new rules and big changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants