Bug 217415 - Presidential Executive Order pages not accessible with Safari.
Summary: Presidential Executive Order pages not accessible with Safari.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andres Gonzalez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-10-06 17:11 PDT by Andres Gonzalez
Modified: 2020-10-08 13:20 PDT (History)
9 users (show)

See Also:


Attachments
Patch (11.90 KB, patch)
2020-10-06 17:25 PDT, Andres Gonzalez
no flags Details | Formatted Diff | Diff
Patch (2.13 KB, patch)
2020-10-07 14:47 PDT, Andres Gonzalez
no flags Details | Formatted Diff | Diff
Patch (2.44 KB, patch)
2020-10-07 16:34 PDT, Andres Gonzalez
no flags Details | Formatted Diff | Diff
Patch (6.06 KB, patch)
2020-10-08 06:14 PDT, Andres Gonzalez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andres Gonzalez 2020-10-06 17:11:43 PDT
Presidential Executive Order pages not accessible with Safari.
Comment 1 Andres Gonzalez 2020-10-06 17:25:13 PDT
Created attachment 410716 [details]
Patch
Comment 2 Andres Gonzalez 2020-10-06 17:28:39 PDT
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.
Comment 3 Andres Gonzalez 2020-10-06 17:29:38 PDT
<rdar://problem/69922416>
Comment 4 EWS 2020-10-06 23:00:03 PDT
Committed r268117: <https://trac.webkit.org/changeset/268117>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 410716 [details].
Comment 5 Simon Fraser (smfr) 2020-10-07 12:35:59 PDT
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.
Comment 6 Andres Gonzalez 2020-10-07 14:47:48 PDT
Reopening to attach new patch.
Comment 7 Andres Gonzalez 2020-10-07 14:47:49 PDT
Created attachment 410785 [details]
Patch
Comment 8 chris fleizach 2020-10-07 14:56:10 PDT
Comment on attachment 410785 [details]
Patch

should we update the test to account for this style of visibility?
Comment 9 Simon Fraser (smfr) 2020-10-07 15:00:57 PDT
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())
Comment 10 Andres Gonzalez 2020-10-07 15:49:55 PDT
(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?
Comment 11 Andres Gonzalez 2020-10-07 16:34:36 PDT
Created attachment 410800 [details]
Patch
Comment 12 Andres Gonzalez 2020-10-07 16:54:22 PDT
(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.
Comment 13 Andres Gonzalez 2020-10-07 17:00:10 PDT
(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.
Comment 14 Simon Fraser (smfr) 2020-10-07 17:01:46 PDT
Opacity multiplies, so a descendant can't be less opaque than its parent.
Comment 15 Andres Gonzalez 2020-10-08 06:14:36 PDT
Created attachment 410834 [details]
Patch
Comment 16 Andres Gonzalez 2020-10-08 06:19:31 PDT
(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.
Comment 17 EWS 2020-10-08 13:20:13 PDT
Committed r268206: <https://trac.webkit.org/changeset/268206>

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