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].