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

Explicit display:none should prevent element from being used as relevant canvas fallback content #7534

Open
Loirooriol opened this issue Jan 25, 2022 · 7 comments
Labels
a11y-tracker Group bringing to attention of a11y, or tracked by the a11y Group but not needing response. accessibility Affects accessibility topic: canvas topic: focus

Comments

@Loirooriol
Copy link
Contributor

https://html.spec.whatwg.org/multipage/canvas.html#being-used-as-relevant-canvas-fallback-content

An element whose nearest canvas element ancestor is being rendered and represents embedded content is an element that is being used as relevant canvas fallback content.

https://html.spec.whatwg.org/multipage/interaction.html#focusable-area

Focusable area Examples
Elements that meet all the following criteria:

  • the element's tabindex value is non-null, or the element is determined by the user agent to be focusable;
  • the element is either not a shadow host, or has a shadow root whose delegates focus is false;
  • the element is not actually disabled;
  • the element is not expressly inert;
  • the element is either being rendered or being used as relevant canvas fallback content.

But let's say you have multiple fallbacks and choose between them with display:none:

<canvas>
  <div id="fallback1" style="display:none">
    <span tabindex="1">Fallback1</span>
  </div>
  <div id="fallback2">
    <span tabindex="1">Fallback2</span>
  </div>
</cavas>

As @lilles told me, in the case where we should render the fallback content, #fallback1>span would not be focusable. It sounds unintentional that all of the fallback content should be focusable if we render the canvas and not render the fallback.

Also, it's already not focusable in Firefox, and I plan to make it not focusable in Blink, since it helps for inert. The spec lists "not expressly inert" as a condition for being focusable, but typically browsers don't compute styles in display: none subtrees. But all Blink, WebKit and Gecko have converged to track inertness in the computed style, so it may not be known if the element is inert or not.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 26, 2022
Descendants of a canvas element aren't typically rendered, so by default
they wouldn't be focusable. But HTML adds an exception: if the nearest
canvas ancestor is being rendered and represents embedded content, the
element can be focusable even if not rendered.

There were some problems with this:

- Blink was always applying the exception, even if the canvas didn't
  represent embedded content.

- Even if the element doesn't need to be rendered, other conditions
  still apply, in particular, not being inert. However, if we have a
  display:none subtree inside a canvas, then usually there is no
  ComputedStyle to check for IsInert().
  The code was assuming that it wouldn't be inert, but it wasn't great:
  computing the style with getComputedStyle() or other JS APIs could
  then make the element be detected as inert, and stop being focusable.

- It's also likely that allowing display:none elements to be focusable
  wasn't really the intent: whatwg/html#7534

This patch changes Element::IsFocusableStyle() so that elements in a
canvas that wouldn't normally be focusable are considered focusable if:
- They have a cached computed style
- They are not in a display:none subtree
- Their computed style allows focus (visible and not inert)
- Their canvas represents embedded content (has LayoutHTMLCanvas layout)
- Their canvas' style allows focus

TEST=AccessibilityTest.CanSetFocusInCanvasFallbackContent
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-001.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-002.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-003.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-004.html
TEST=external/wpt/inert/inert-canvas-fallback-content.tentative.html

Bug: 692360
Change-Id: Iae979322ac32df7e8ea68255be13be0419c9fc93
Cq-Do-Not-Cancel-Tryjobs: true
@Loirooriol
Copy link
Contributor Author

Same for display: contents and not being in the flat tree.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 26, 2022
Descendants of a canvas element aren't typically rendered, so by default
they wouldn't be focusable. But HTML adds an exception: if the nearest
canvas ancestor is being rendered and represents embedded content, the
element can be focusable even if not rendered.

There were some problems with this:

- Blink was always applying the exception, even if the canvas didn't
  represent embedded content.

- Even if the element doesn't need to be rendered, other conditions
  still apply, in particular, not being inert. However, if we have a
  display:none subtree inside a canvas, then usually there is no
  ComputedStyle to check for IsInert().
  The code was assuming that it wouldn't be inert, but it wasn't great:
  computing the style with getComputedStyle() or other JS APIs could
  then make the element be detected as inert, and stop being focusable.

- It's also likely that allowing display:none elements to be focusable
  wasn't really the intent: whatwg/html#7534

This patch changes Element::IsFocusableStyle() so that elements in a
canvas that wouldn't normally be focusable are considered focusable if:
- They have a cached computed style
- They are not in a display:none subtree
- Their computed style allows focus (visible and not inert)
- Their canvas represents embedded content (has LayoutHTMLCanvas layout)
- Their canvas' style allows focus

TEST=AccessibilityTest.CanSetFocusInCanvasFallbackContent
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-001.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-002.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-003.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-004.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-005.html
TEST=external/wpt/inert/inert-canvas-fallback-content.tentative.html

Bug: 692360
Change-Id: Iae979322ac32df7e8ea68255be13be0419c9fc93
Cq-Do-Not-Cancel-Tryjobs: true
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 26, 2022
Descendants of a canvas element aren't typically rendered, so by default
they wouldn't be focusable. But HTML adds an exception: if the nearest
canvas ancestor is being rendered and represents embedded content, the
element can be focusable even if not rendered.

There were some problems with this:

- Blink was always applying the exception, even if the canvas didn't
  represent embedded content.

- Even if the element doesn't need to be rendered, other conditions
  still apply, in particular, not being inert. However, if we have a
  display:none subtree inside a canvas, then usually there is no
  ComputedStyle to check for IsInert().
  The code was assuming that it wouldn't be inert, but it wasn't great:
  computing the style with getComputedStyle() or other JS APIs could
  then make the element be detected as inert, and stop being focusable.

- It's also likely that allowing display:none elements to be focusable
  wasn't really the intent: whatwg/html#7534

This patch changes Element::IsFocusableStyle() so that elements in a
canvas that wouldn't normally be focusable are considered focusable if:
- They have a cached computed style
- They are not in a display:none subtree, nor have display:contents.
- Their computed style allows focus (visible and not inert)
- Their canvas represents embedded content (has LayoutHTMLCanvas layout)
- Their canvas' style allows focus

TEST=AccessibilityTest.CanSetFocusInCanvasFallbackContent
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-001.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-002.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-003.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-004.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-005.html
TEST=external/wpt/inert/inert-canvas-fallback-content.tentative.html

Bug: 692360
Change-Id: Iae979322ac32df7e8ea68255be13be0419c9fc93
Cq-Do-Not-Cancel-Tryjobs: true
aarongable pushed a commit to chromium/chromium that referenced this issue Jan 26, 2022
Descendants of a canvas element aren't typically rendered, so by default
they wouldn't be focusable. But HTML adds an exception: if the nearest
canvas ancestor is being rendered and represents embedded content, the
element can be focusable even if not rendered.

There were some problems with this:

- Blink was always applying the exception, even if the canvas didn't
  represent embedded content.

- Even if the element doesn't need to be rendered, other conditions
  still apply, in particular, not being inert. However, if we have a
  display:none subtree inside a canvas, then usually there is no
  ComputedStyle to check for IsInert().
  The code was assuming that it wouldn't be inert, but it wasn't great:
  computing the style with getComputedStyle() or other JS APIs could
  then make the element be detected as inert, and stop being focusable.

- It's also likely that allowing display:none elements to be focusable
  wasn't really the intent: whatwg/html#7534

This patch changes Element::IsFocusableStyle() so that elements in a
canvas that wouldn't normally be focusable are considered focusable if:
- They have a cached computed style
- They are not in a display:none subtree, nor have display:contents.
- Their computed style allows focus (visible and not inert)
- Their canvas represents embedded content (has LayoutHTMLCanvas layout)
- Their canvas' style allows focus

TEST=AccessibilityTest.CanSetFocusInCanvasFallbackContent
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-001.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-002.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-003.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-004.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-005.html
TEST=external/wpt/inert/inert-canvas-fallback-content.tentative.html

Bug: 692360
Change-Id: Iae979322ac32df7e8ea68255be13be0419c9fc93
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3395837
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/main@{#963655}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 26, 2022
Descendants of a canvas element aren't typically rendered, so by default
they wouldn't be focusable. But HTML adds an exception: if the nearest
canvas ancestor is being rendered and represents embedded content, the
element can be focusable even if not rendered.

There were some problems with this:

- Blink was always applying the exception, even if the canvas didn't
  represent embedded content.

- Even if the element doesn't need to be rendered, other conditions
  still apply, in particular, not being inert. However, if we have a
  display:none subtree inside a canvas, then usually there is no
  ComputedStyle to check for IsInert().
  The code was assuming that it wouldn't be inert, but it wasn't great:
  computing the style with getComputedStyle() or other JS APIs could
  then make the element be detected as inert, and stop being focusable.

- It's also likely that allowing display:none elements to be focusable
  wasn't really the intent: whatwg/html#7534

This patch changes Element::IsFocusableStyle() so that elements in a
canvas that wouldn't normally be focusable are considered focusable if:
- They have a cached computed style
- They are not in a display:none subtree, nor have display:contents.
- Their computed style allows focus (visible and not inert)
- Their canvas represents embedded content (has LayoutHTMLCanvas layout)
- Their canvas' style allows focus

TEST=AccessibilityTest.CanSetFocusInCanvasFallbackContent
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-001.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-002.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-003.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-004.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-005.html
TEST=external/wpt/inert/inert-canvas-fallback-content.tentative.html

Bug: 692360
Change-Id: Iae979322ac32df7e8ea68255be13be0419c9fc93
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3395837
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/main@{#963655}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 26, 2022
Descendants of a canvas element aren't typically rendered, so by default
they wouldn't be focusable. But HTML adds an exception: if the nearest
canvas ancestor is being rendered and represents embedded content, the
element can be focusable even if not rendered.

There were some problems with this:

- Blink was always applying the exception, even if the canvas didn't
  represent embedded content.

- Even if the element doesn't need to be rendered, other conditions
  still apply, in particular, not being inert. However, if we have a
  display:none subtree inside a canvas, then usually there is no
  ComputedStyle to check for IsInert().
  The code was assuming that it wouldn't be inert, but it wasn't great:
  computing the style with getComputedStyle() or other JS APIs could
  then make the element be detected as inert, and stop being focusable.

- It's also likely that allowing display:none elements to be focusable
  wasn't really the intent: whatwg/html#7534

This patch changes Element::IsFocusableStyle() so that elements in a
canvas that wouldn't normally be focusable are considered focusable if:
- They have a cached computed style
- They are not in a display:none subtree, nor have display:contents.
- Their computed style allows focus (visible and not inert)
- Their canvas represents embedded content (has LayoutHTMLCanvas layout)
- Their canvas' style allows focus

TEST=AccessibilityTest.CanSetFocusInCanvasFallbackContent
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-001.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-002.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-003.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-004.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-005.html
TEST=external/wpt/inert/inert-canvas-fallback-content.tentative.html

Bug: 692360
Change-Id: Iae979322ac32df7e8ea68255be13be0419c9fc93
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3395837
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/main@{#963655}
@annevk annevk added a11y-tracker Group bringing to attention of a11y, or tracked by the a11y Group but not needing response. accessibility Affects accessibility topic: canvas labels Feb 15, 2022
@annevk
Copy link
Member

annevk commented Feb 15, 2022

That seems reasonable, but I'm wondering how this works technically. Do you essentially try to render the fallback contents to determine what would and would not render?

This seems related to #7490.

cc @whatwg/a11y @whatwg/canvas

@patrickhlauke
Copy link
Member

naive question: would this then hinge not on "is being rendered" but on a more abstract "has been flagged as not rendered/renderable through the use of display:none or similar" (i.e. is there some kind of "state" or flag that comes before the browser's decision to render or not render something, and if so should it lean on that rather than "is being rendered" which is the outcome of that evaluation)

@Loirooriol
Copy link
Contributor Author

Do you essentially try to render the fallback contents to determine what would and would not render?

No. In Blink, elements in display: none typically don't have a cached ComputedStyle, unless e.g. a script uses getComputedStyle or something to force it. So what I did is: if the element doesn't have a cached computed style, or if it has the IsEnsuredInDisplayNone() flag, or if has display: contents, then it's not focusable.

Note this is an approximation, since e.g. in the canvas fallback content there could be a replaced element with a child:

canvas.innerHTML = "<img src='image'>";
var div = document.createElement("div");
div.tabIndex = -1;
canvas.firstElementChild.append(div);
div.focus();

Even when rendering the canvas fallback content, the div wouldn't be rendered, but it's still focusable, even if it wouldn't be focusable outside of a canvas.

naive question: would this then hinge not on "is being rendered" but on a more abstract "has been flagged as not rendered/renderable through the use of display:none or similar"

That's the case when inside a canvas. Otherwise Blink uses "is being rendered". It may be possible to have a more consistent behavior.

@annevk
Copy link
Member

annevk commented Feb 16, 2022

Interesting, I would have expected fallback content to be treated similarly to display:none as it cannot appear on screen. I guess we can define something along the lines you suggest.

cc @whatwg/css @emilio

@Loirooriol
Copy link
Contributor Author

Treating it like display: none would make canvas fallback content not focusable at all. Which would be another possibility, but seems a riskier change. I basically did the minimal change to consistently handle inertness (it's tracked in the computed style, so it's a problem if elements in display: none that don't have a cached computed style can be focused).

@junov
Copy link
Member

junov commented Feb 16, 2022

Se the discussion on this other closely related issue (already mentioned above). We may want to revisit the interactiveness of fallback content.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 26, 2022
…ndants, a=testonly

Automatic update from web-platform-tests
[inert] Fix focusability of canvas descendants

Descendants of a canvas element aren't typically rendered, so by default
they wouldn't be focusable. But HTML adds an exception: if the nearest
canvas ancestor is being rendered and represents embedded content, the
element can be focusable even if not rendered.

There were some problems with this:

- Blink was always applying the exception, even if the canvas didn't
  represent embedded content.

- Even if the element doesn't need to be rendered, other conditions
  still apply, in particular, not being inert. However, if we have a
  display:none subtree inside a canvas, then usually there is no
  ComputedStyle to check for IsInert().
  The code was assuming that it wouldn't be inert, but it wasn't great:
  computing the style with getComputedStyle() or other JS APIs could
  then make the element be detected as inert, and stop being focusable.

- It's also likely that allowing display:none elements to be focusable
  wasn't really the intent: whatwg/html#7534

This patch changes Element::IsFocusableStyle() so that elements in a
canvas that wouldn't normally be focusable are considered focusable if:
- They have a cached computed style
- They are not in a display:none subtree, nor have display:contents.
- Their computed style allows focus (visible and not inert)
- Their canvas represents embedded content (has LayoutHTMLCanvas layout)
- Their canvas' style allows focus

TEST=AccessibilityTest.CanSetFocusInCanvasFallbackContent
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-001.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-002.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-003.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-004.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-005.html
TEST=external/wpt/inert/inert-canvas-fallback-content.tentative.html

Bug: 692360
Change-Id: Iae979322ac32df7e8ea68255be13be0419c9fc93
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3395837
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/main@{#963655}

--

wpt-commits: 0dd1fbb1e02af7525bc4918455962194686ddb97
wpt-pr: 32494
DanielRyanSmith pushed a commit to DanielRyanSmith/wpt that referenced this issue Feb 28, 2022
Descendants of a canvas element aren't typically rendered, so by default
they wouldn't be focusable. But HTML adds an exception: if the nearest
canvas ancestor is being rendered and represents embedded content, the
element can be focusable even if not rendered.

There were some problems with this:

- Blink was always applying the exception, even if the canvas didn't
  represent embedded content.

- Even if the element doesn't need to be rendered, other conditions
  still apply, in particular, not being inert. However, if we have a
  display:none subtree inside a canvas, then usually there is no
  ComputedStyle to check for IsInert().
  The code was assuming that it wouldn't be inert, but it wasn't great:
  computing the style with getComputedStyle() or other JS APIs could
  then make the element be detected as inert, and stop being focusable.

- It's also likely that allowing display:none elements to be focusable
  wasn't really the intent: whatwg/html#7534

This patch changes Element::IsFocusableStyle() so that elements in a
canvas that wouldn't normally be focusable are considered focusable if:
- They have a cached computed style
- They are not in a display:none subtree, nor have display:contents.
- Their computed style allows focus (visible and not inert)
- Their canvas represents embedded content (has LayoutHTMLCanvas layout)
- Their canvas' style allows focus

TEST=AccessibilityTest.CanSetFocusInCanvasFallbackContent
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-001.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-002.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-003.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-004.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-005.html
TEST=external/wpt/inert/inert-canvas-fallback-content.tentative.html

Bug: 692360
Change-Id: Iae979322ac32df7e8ea68255be13be0419c9fc93
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3395837
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/main@{#963655}
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Mar 1, 2022
…ndants, a=testonly

Automatic update from web-platform-tests
[inert] Fix focusability of canvas descendants

Descendants of a canvas element aren't typically rendered, so by default
they wouldn't be focusable. But HTML adds an exception: if the nearest
canvas ancestor is being rendered and represents embedded content, the
element can be focusable even if not rendered.

There were some problems with this:

- Blink was always applying the exception, even if the canvas didn't
  represent embedded content.

- Even if the element doesn't need to be rendered, other conditions
  still apply, in particular, not being inert. However, if we have a
  display:none subtree inside a canvas, then usually there is no
  ComputedStyle to check for IsInert().
  The code was assuming that it wouldn't be inert, but it wasn't great:
  computing the style with getComputedStyle() or other JS APIs could
  then make the element be detected as inert, and stop being focusable.

- It's also likely that allowing display:none elements to be focusable
  wasn't really the intent: whatwg/html#7534

This patch changes Element::IsFocusableStyle() so that elements in a
canvas that wouldn't normally be focusable are considered focusable if:
- They have a cached computed style
- They are not in a display:none subtree, nor have display:contents.
- Their computed style allows focus (visible and not inert)
- Their canvas represents embedded content (has LayoutHTMLCanvas layout)
- Their canvas' style allows focus

TEST=AccessibilityTest.CanSetFocusInCanvasFallbackContent
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-001.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-002.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-003.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-004.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-005.html
TEST=external/wpt/inert/inert-canvas-fallback-content.tentative.html

Bug: 692360
Change-Id: Iae979322ac32df7e8ea68255be13be0419c9fc93
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3395837
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/main@{#963655}

--

wpt-commits: 0dd1fbb1e02af7525bc4918455962194686ddb97
wpt-pr: 32494
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Mar 8, 2022
…ndants, a=testonly

Automatic update from web-platform-tests
[inert] Fix focusability of canvas descendants

Descendants of a canvas element aren't typically rendered, so by default
they wouldn't be focusable. But HTML adds an exception: if the nearest
canvas ancestor is being rendered and represents embedded content, the
element can be focusable even if not rendered.

There were some problems with this:

- Blink was always applying the exception, even if the canvas didn't
  represent embedded content.

- Even if the element doesn't need to be rendered, other conditions
  still apply, in particular, not being inert. However, if we have a
  display:none subtree inside a canvas, then usually there is no
  ComputedStyle to check for IsInert().
  The code was assuming that it wouldn't be inert, but it wasn't great:
  computing the style with getComputedStyle() or other JS APIs could
  then make the element be detected as inert, and stop being focusable.

- It's also likely that allowing display:none elements to be focusable
  wasn't really the intent: whatwg/html#7534

This patch changes Element::IsFocusableStyle() so that elements in a
canvas that wouldn't normally be focusable are considered focusable if:
- They have a cached computed style
- They are not in a display:none subtree, nor have display:contents.
- Their computed style allows focus (visible and not inert)
- Their canvas represents embedded content (has LayoutHTMLCanvas layout)
- Their canvas' style allows focus

TEST=AccessibilityTest.CanSetFocusInCanvasFallbackContent
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-001.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-002.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-003.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-004.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-005.html
TEST=external/wpt/inert/inert-canvas-fallback-content.tentative.html

Bug: 692360
Change-Id: Iae979322ac32df7e8ea68255be13be0419c9fc93
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3395837
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/main@{#963655}

--

wpt-commits: 0dd1fbb1e02af7525bc4918455962194686ddb97
wpt-pr: 32494
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Mar 8, 2022
…ndants, a=testonly

Automatic update from web-platform-tests
[inert] Fix focusability of canvas descendants

Descendants of a canvas element aren't typically rendered, so by default
they wouldn't be focusable. But HTML adds an exception: if the nearest
canvas ancestor is being rendered and represents embedded content, the
element can be focusable even if not rendered.

There were some problems with this:

- Blink was always applying the exception, even if the canvas didn't
  represent embedded content.

- Even if the element doesn't need to be rendered, other conditions
  still apply, in particular, not being inert. However, if we have a
  display:none subtree inside a canvas, then usually there is no
  ComputedStyle to check for IsInert().
  The code was assuming that it wouldn't be inert, but it wasn't great:
  computing the style with getComputedStyle() or other JS APIs could
  then make the element be detected as inert, and stop being focusable.

- It's also likely that allowing display:none elements to be focusable
  wasn't really the intent: whatwg/html#7534

This patch changes Element::IsFocusableStyle() so that elements in a
canvas that wouldn't normally be focusable are considered focusable if:
- They have a cached computed style
- They are not in a display:none subtree, nor have display:contents.
- Their computed style allows focus (visible and not inert)
- Their canvas represents embedded content (has LayoutHTMLCanvas layout)
- Their canvas' style allows focus

TEST=AccessibilityTest.CanSetFocusInCanvasFallbackContent
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-001.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-002.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-003.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-004.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-005.html
TEST=external/wpt/inert/inert-canvas-fallback-content.tentative.html

Bug: 692360
Change-Id: Iae979322ac32df7e8ea68255be13be0419c9fc93
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3395837
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/main@{#963655}

--

wpt-commits: 0dd1fbb1e02af7525bc4918455962194686ddb97
wpt-pr: 32494
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Descendants of a canvas element aren't typically rendered, so by default
they wouldn't be focusable. But HTML adds an exception: if the nearest
canvas ancestor is being rendered and represents embedded content, the
element can be focusable even if not rendered.

There were some problems with this:

- Blink was always applying the exception, even if the canvas didn't
  represent embedded content.

- Even if the element doesn't need to be rendered, other conditions
  still apply, in particular, not being inert. However, if we have a
  display:none subtree inside a canvas, then usually there is no
  ComputedStyle to check for IsInert().
  The code was assuming that it wouldn't be inert, but it wasn't great:
  computing the style with getComputedStyle() or other JS APIs could
  then make the element be detected as inert, and stop being focusable.

- It's also likely that allowing display:none elements to be focusable
  wasn't really the intent: whatwg/html#7534

This patch changes Element::IsFocusableStyle() so that elements in a
canvas that wouldn't normally be focusable are considered focusable if:
- They have a cached computed style
- They are not in a display:none subtree, nor have display:contents.
- Their computed style allows focus (visible and not inert)
- Their canvas represents embedded content (has LayoutHTMLCanvas layout)
- Their canvas' style allows focus

TEST=AccessibilityTest.CanSetFocusInCanvasFallbackContent
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-001.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-002.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-003.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-004.tentative.html
TEST=external/wpt/html/semantics/embedded-content/the-canvas-element/canvas-descendants-focusability-005.html
TEST=external/wpt/inert/inert-canvas-fallback-content.tentative.html

Bug: 692360
Change-Id: Iae979322ac32df7e8ea68255be13be0419c9fc93
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3395837
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/main@{#963655}
NOKEYCHECK=True
GitOrigin-RevId: 45a69b59e224832a4481c10b4a2a1e2f28db1743
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y-tracker Group bringing to attention of a11y, or tracked by the a11y Group but not needing response. accessibility Affects accessibility topic: canvas topic: focus
Development

No branches or pull requests

4 participants