Bug 217415

Created attachment 410716 [details]
Patch

This page is totally inaccessible with VoiceOver using Safari 14.

To reproduce:
1. Run VoiceOver and go to 
https://www.whitehouse.gov/presidential-actions/executive-order-combating-race-sex-stereotyping/?fbclid=IwAR1yqUwgQWB1oppxvi4Jul7sO_Sz5mbbvwmzm8uCDat12df-NUU1ePldmMg
2. Try to read the page content using VoiceOver standard reading and navigation keystrokes.
3. VoiceOver does not read any of the page content.

<rdar://problem/69922416>

Committed r268117: <https://trac.webkit.org/changeset/268117>

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

Comment on attachment 410716 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=410716&action=review

> Source/WebCore/accessibility/AXObjectCache.cpp:305
> +        || style.visibility() != Visibility::Visible

This check is wrong. You also have to consult visibility on ancestors.

> Source/WebCore/accessibility/AXObjectCache.cpp:306
> +        || !style.opacity())

This check is wrong. You also have to consult opacity on ancestors.

Reopening to attach new patch.

Created attachment 410785 [details]
Patch

Comment on attachment 410785 [details]
Patch

should we update the test to account for this style of visibility?

Comment on attachment 410785 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=410785&action=review

> Source/WebCore/accessibility/AXObjectCache.cpp:310
> +        if (style.visibility() != Visibility::Visible || !style.opacity())
> +            return false;

This is also wrong :)

A descendant can set visibility:visible to override visibility:hidden on an ancestor.

You really have to ask these questions of the renderer. Right now we don't have a good way to do that, but this gets close:

    if (style().visibility() != Visibility::Visible && !enclosingLayer()->hasVisibleContent())

(In reply to Simon Fraser (smfr) from comment #9)
> Comment on attachment 410785 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=410785&action=review
> 
> > Source/WebCore/accessibility/AXObjectCache.cpp:310
> > +        if (style.visibility() != Visibility::Visible || !style.opacity())
> > +            return false;
> 
> This is also wrong :)
> 
> A descendant can set visibility:visible to override visibility:hidden on an
> ancestor.
> 
> You really have to ask these questions of the renderer. Right now we don't
> have a good way to do that, but this gets close:
> 
>     if (style().visibility() != Visibility::Visible &&
> !enclosingLayer()->hasVisibleContent())

What about the opacity? can it be also overridden?

Created attachment 410800 [details]
Patch

(In reply to Simon Fraser (smfr) from comment #9)
> Comment on attachment 410785 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=410785&action=review
> 
> > Source/WebCore/accessibility/AXObjectCache.cpp:310
> > +        if (style.visibility() != Visibility::Visible || !style.opacity())
> > +            return false;
> 
> This is also wrong :)
> 
> A descendant can set visibility:visible to override visibility:hidden on an
> ancestor.
> 
> You really have to ask these questions of the renderer. Right now we don't
> have a good way to do that, but this gets close:
> 
>     if (style().visibility() != Visibility::Visible &&
> !enclosingLayer()->hasVisibleContent())

OK, done in latest patch. Still the question about whether opacity can be overridden by a descendant.

(In reply to chris fleizach from comment #8)
> Comment on attachment 410785 [details]
> Patch
> 
> should we update the test to account for this style of visibility?

Will do if the opacity can't be overridden by a descendant. Otherwise the change to the opacity check makes no sense.

Opacity multiplies, so a descendant can't be less opaque than its parent.

Created attachment 410834 [details]
Patch

(In reply to chris fleizach from comment #8)
> Comment on attachment 410785 [details]
> Patch
> 
> should we update the test to account for this style of visibility?

Added the test case for the opacity of the parent.

Committed r268206: <https://trac.webkit.org/changeset/268206>

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