[APC] Obey aria-hidden [chromium/src : main]
Aaron Leventhal (Gerrit)
Aaron Leventhal voted and added 1 comment![]( "Open in Gerrit")
Votes added by Aaron Leventhal
| Commit-Queue | +1 |
1 comment
Khushal, should we use disabled reasons for these two CLs so that they can be in the actionable list but marked disabled?
Related details
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Khushal Sagar (Gerrit)
Khushal Sagar added 1 comment![]( "Open in Gerrit")
// aria-hidden prunes the entire subtree rooted at this child, so a localIs content inside aria-hidden findable? We've had a principle of keeping content which the user can get to via find-in-page in the tree. So I'm wondering if removing the content is the right approach, or do we just want to make this content not actionable.
Related details
- Aaron Leventhal
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aaron Leventhal (Gerrit)
Aaron Leventhal added 1 comment![]( "Open in Gerrit")
// aria-hidden prunes the entire subtree rooted at this child, so a localIs content inside aria-hidden findable? We've had a principle of keeping content which the user can get to via find-in-page in the tree. So I'm wondering if removing the content is the right approach, or do we just want to make this content not actionable.
It's findable. One iron clad rule is that ARIA does not affect the rendering or behavior of a page in the browser rendering engine. It's the author signaling what their intentions are. IMO it's bad to go against the author's intentions.
I propose we do this instead, for all nodes in an aria-hidden subtree:
- Mark anything with interaction info as having a DisabledReasons::kAriaHidden (will add this)
- Use the kAIPageContentAnnotatedRole::kContentHidden
In addition, role=none/presentation would also get is own disabled reasons.
This way we respect the author's intentions.
Related details
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Khushal Sagar (Gerrit)
Khushal Sagar added 1 comment![]( "Open in Gerrit")
// aria-hidden prunes the entire subtree rooted at this child, so a localAaron LeventhalIs content inside aria-hidden findable? We've had a principle of keeping content which the user can get to via find-in-page in the tree. So I'm wondering if removing the content is the right approach, or do we just want to make this content not actionable.
It's findable. One iron clad rule is that ARIA does not affect the rendering or behavior of a page in the browser rendering engine. It's the author signaling what their intentions are. IMO it's bad to go against the author's intentions.
I propose we do this instead, for all nodes in an aria-hidden subtree:
- Mark anything with interaction info as having a DisabledReasons::kAriaHidden (will add this)
- Use the kAIPageContentAnnotatedRole::kContentHidden
In addition, role=none/presentation would also get is own disabled reasons.
This way we respect the author's intentions.
Mark anything with interaction info as having a DisabledReasons::kAriaHidden
Is the content hit-testable? Since you said aria doesn't change behaviour of the page, i want to confirm the behaviour for both: hit-testability (will it consume events) and interactibility (do click handlers etc fire if the attribute is set).
Use the kAIPageContentAnnotatedRole::kContentHidden
This I'm not sure about. This API is currently being used for cases where the content is visually hidden from the user but find-in-page can reveal it. What are the semantics with aria-hidden? Based on what you said, sounds like it doesn't affect rendering so the user can still see this content.
Related details
- Aaron Leventhal
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aaron Leventhal (Gerrit)
Aaron Leventhal added 1 comment![]( "Open in Gerrit")
// aria-hidden prunes the entire subtree rooted at this child, so a localAaron LeventhalIs content inside aria-hidden findable? We've had a principle of keeping content which the user can get to via find-in-page in the tree. So I'm wondering if removing the content is the right approach, or do we just want to make this content not actionable.
Khushal SagarIt's findable. One iron clad rule is that ARIA does not affect the rendering or behavior of a page in the browser rendering engine. It's the author signaling what their intentions are. IMO it's bad to go against the author's intentions.
I propose we do this instead, for all nodes in an aria-hidden subtree:
- Mark anything with interaction info as having a DisabledReasons::kAriaHidden (will add this)
- Use the kAIPageContentAnnotatedRole::kContentHidden
In addition, role=none/presentation would also get is own disabled reasons.
This way we respect the author's intentions.
Mark anything with interaction info as having a DisabledReasons::kAriaHidden
Is the content hit-testable? Since you said aria doesn't change behaviour of the page, i want to confirm the behaviour for both: hit-testability (will it consume events) and interactibility (do click handlers etc fire if the attribute is set).
Use the kAIPageContentAnnotatedRole::kContentHidden
This I'm not sure about. This API is currently being used for cases where the content is visually hidden from the user but find-in-page can reveal it. What are the semantics with aria-hidden? Based on what you said, sounds like it doesn't affect rendering so the user can still see this content.
Yes it is hit testable and findable, just like aria-disabled.
However it is not findable or usable in a screen reader or assistive technology.
ARIA doesn't drive behavior, it describes author intent.
Look at how we handle aria-disabled now. My vote is to respect the author's wishes.
Related details
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Khushal Sagar (Gerrit)
Khushal Sagar added 1 comment![]( "Open in Gerrit")
// aria-hidden prunes the entire subtree rooted at this child, so a localAaron LeventhalIs content inside aria-hidden findable? We've had a principle of keeping content which the user can get to via find-in-page in the tree. So I'm wondering if removing the content is the right approach, or do we just want to make this content not actionable.
Khushal SagarIt's findable. One iron clad rule is that ARIA does not affect the rendering or behavior of a page in the browser rendering engine. It's the author signaling what their intentions are. IMO it's bad to go against the author's intentions.
I propose we do this instead, for all nodes in an aria-hidden subtree:
- Mark anything with interaction info as having a DisabledReasons::kAriaHidden (will add this)
- Use the kAIPageContentAnnotatedRole::kContentHidden
In addition, role=none/presentation would also get is own disabled reasons.
This way we respect the author's intentions.
Aaron LeventhalMark anything with interaction info as having a DisabledReasons::kAriaHidden
Is the content hit-testable? Since you said aria doesn't change behaviour of the page, i want to confirm the behaviour for both: hit-testability (will it consume events) and interactibility (do click handlers etc fire if the attribute is set).
Use the kAIPageContentAnnotatedRole::kContentHidden
This I'm not sure about. This API is currently being used for cases where the content is visually hidden from the user but find-in-page can reveal it. What are the semantics with aria-hidden? Based on what you said, sounds like it doesn't affect rendering so the user can still see this content.
Yes it is hit testable and findable, just like aria-disabled.
However it is not findable or usable in a screen reader or assistive technology.ARIA doesn't drive behavior, it describes author intent.
Look at how we handle aria-disabled now. My vote is to respect the author's wishes.
Cool. Agreed then, lets keep the behaviour we have with aria-disabled but with a new Reason. That still keeps the content in the tree and marks it hit-testable.
Related details
- Aaron Leventhal
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aaron Leventhal (Gerrit)
Aaron Leventhal added 1 comment![]( "Open in Gerrit")
// aria-hidden prunes the entire subtree rooted at this child, so a localAaron LeventhalIs content inside aria-hidden findable? We've had a principle of keeping content which the user can get to via find-in-page in the tree. So I'm wondering if removing the content is the right approach, or do we just want to make this content not actionable.
Khushal SagarIt's findable. One iron clad rule is that ARIA does not affect the rendering or behavior of a page in the browser rendering engine. It's the author signaling what their intentions are. IMO it's bad to go against the author's intentions.
I propose we do this instead, for all nodes in an aria-hidden subtree:
- Mark anything with interaction info as having a DisabledReasons::kAriaHidden (will add this)
- Use the kAIPageContentAnnotatedRole::kContentHidden
In addition, role=none/presentation would also get is own disabled reasons.
This way we respect the author's intentions.
Aaron LeventhalMark anything with interaction info as having a DisabledReasons::kAriaHidden
Is the content hit-testable? Since you said aria doesn't change behaviour of the page, i want to confirm the behaviour for both: hit-testability (will it consume events) and interactibility (do click handlers etc fire if the attribute is set).
Use the kAIPageContentAnnotatedRole::kContentHidden
This I'm not sure about. This API is currently being used for cases where the content is visually hidden from the user but find-in-page can reveal it. What are the semantics with aria-hidden? Based on what you said, sounds like it doesn't affect rendering so the user can still see this content.
Khushal SagarYes it is hit testable and findable, just like aria-disabled.
However it is not findable or usable in a screen reader or assistive technology.ARIA doesn't drive behavior, it describes author intent.
Look at how we handle aria-disabled now. My vote is to respect the author's wishes.
Cool. Agreed then, lets keep the behaviour we have with aria-disabled but with a new Reason. That still keeps the content in the tree and marks it hit-testable.
- Khushal Sagar
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aaron Leventhal (Gerrit)
Aaron Leventhal removed Abigail Klein from this change![]( "Open in Gerrit")
- Chromium IPC Reviews
- Khushal Sagar
- Xiaochen Zhou
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: d...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): d...@chromium.org
Reviewer source(s):
d...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Dominic Farolino
- Khushal Sagar
- Xiaochen Zhou
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominic Farolino (Gerrit)
Dominic Farolino voted Code-Review+1![]( "Open in Gerrit")
- Aaron Leventhal
- Khushal Sagar
- Xiaochen Zhou
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Khushal Sagar (Gerrit)
Khushal Sagar added 3 comments![]( "Open in Gerrit")
if (is_aria_hidden) {Same here, this is worth taking through an eval to ensure no regression before changing behaviour.
if (recursion_data.is_aria_hidden &&
!attributes.annotated_roles.Contains(
mojom::blink::AIPageContentAnnotatedRole::kContentHidden)) {
attributes.annotated_roles.push_back(
mojom::blink::AIPageContentAnnotatedRole::kContentHidden);
}`kContentHidden` is specifically used for content which is not in layout but can be revealed if the user searches for it. Does aria-hidden have the same semantics? If not, we shouldn't be adding this role.
if (const auto* element = DynamicTo<Element>(object.GetNode());See comment above about not using this annotation for aria-hidden.
Related details
- Aaron Leventhal
- Xiaochen Zhou
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Xiaochen Zhou (Gerrit)
Xiaochen Zhou added 1 comment![]( "Open in Gerrit")
if (is_aria_hidden) {Same here, this is worth taking through an eval to ensure no regression before changing behaviour.
+1. Maybe consider gating the change behind a default-off feature.
Related details
- Aaron Leventhal