Bug 241022 - AX: inert attribute doesn't cause display:contents element to be ignored
Summary: AX: inert attribute doesn't cause display:contents element to be ignored
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tyler Wilcock
URL:
Keywords: InRadar
Depends on:
Blocks: 165279
  Show dependency treegraph
 
Reported: 2022-05-27 08:32 PDT by Tyler Wilcock
Modified: 2022-06-01 09:20 PDT (History)
11 users (show)

See Also:


Attachments
Patch (7.39 KB, patch)
2022-05-27 08:40 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (7.37 KB, patch)
2022-05-31 15:42 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (8.25 KB, patch)
2022-05-31 17:28 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (8.25 KB, patch)
2022-05-31 17:37 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tyler Wilcock 2022-05-27 08:32:47 PDT
AX: inert attribute doesn't cause display:contents element to be ignored
Comment 1 Radar WebKit Bug Importer 2022-05-27 08:32:56 PDT
<rdar://problem/94034984>
Comment 2 Tyler Wilcock 2022-05-27 08:40:27 PDT
Created attachment 459814 [details]
Patch
Comment 3 Tyler Wilcock 2022-05-31 15:42:10 PDT
Created attachment 459914 [details]
Patch
Comment 4 Andres Gonzalez 2022-05-31 17:26:52 PDT
(In reply to Tyler Wilcock from comment #3)
> Created attachment 459914 [details]
> Patch

--- a/Source/WebCore/accessibility/AccessibilityObject.cpp
+++ a/Source/WebCore/accessibility/AccessibilityObject.cpp

+const RenderStyle* AccessibilityObject::style() const

Doesn't this belong to the AXNodeObject since it requires an underlying element? I.e., AccessibilityObject::style should always return nullptr, and AccessibilityNodeObject::style should be this implementation?

--- a/LayoutTests/accessibility/node-only-inert-object.html
+++ a/LayoutTests/accessibility/node-only-inert-object.html

+            await waitFor(() => accessibilityController.accessibleElementById("button") );

Is the space before the last ) needed?
Comment 5 Tyler Wilcock 2022-05-31 17:28:07 PDT
Created attachment 459915 [details]
Patch
Comment 6 Tyler Wilcock 2022-05-31 17:37:05 PDT
(In reply to Andres Gonzalez from comment #4)
> (In reply to Tyler Wilcock from comment #3)
> > Created attachment 459914 [details]
> > Patch
> 
> --- a/Source/WebCore/accessibility/AccessibilityObject.cpp
> +++ a/Source/WebCore/accessibility/AccessibilityObject.cpp
> 
> +const RenderStyle* AccessibilityObject::style() const
> 
> Doesn't this belong to the AXNodeObject since it requires an underlying
> element? I.e., AccessibilityObject::style should always return nullptr, and
> AccessibilityNodeObject::style should be this implementation?
I don't think so, since it uses AccessibilityObject::element() to get the style. Non AXNodeObjects can and do provide a node() / element() / style(). For example, AccessibilityListBoxOption is not an AXNodeObject but provides a node().

Another theoretical example: AccessibilityScrollView isn't a node object, but could provide a node() / element() through AccessibilityScrollView::m_frameOwnerElement.

So I think letting different AccessibilityObject subclasses provide node() / element() / style() however they can is OK.

> --- a/LayoutTests/accessibility/node-only-inert-object.html
> +++ a/LayoutTests/accessibility/node-only-inert-object.html
> 
> +            await waitFor(() =>
> accessibilityController.accessibleElementById("button") );
> 
> Is the space before the last ) needed?
It isn't — fixed.
Comment 7 Tyler Wilcock 2022-05-31 17:37:59 PDT
Created attachment 459916 [details]
Patch
Comment 8 EWS 2022-06-01 09:20:57 PDT
Committed r295090 (251185@main): <https://commits.webkit.org/251185@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 459916 [details].