Bug 158693

Summary: AX: Headers of table not read by VoiceOver
Product: WebKit Reporter: helene.vinh
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Status: RESOLVED FIXED    
Severity: Critical CC: aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, jcraig, jdiggs, mario, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 9   
Hardware: All   
OS: OS X 10.9   
Attachments:
Description Flags
Table extracted from test page
none
Header row is zero sized
none
patch
darin: review+
patch none

Description helene.vinh 2016-06-13 08:40:06 PDT
Created attachment 281175 [details]
Table extracted from test page

Hello,

I am working for Amadeus, a company that provides IT solutions for the travel industry.
We are developing a flight booking website that should be accessible to screen reader users.

We are currently encountering a specific issue with VoiceOver about the headers of a table that are not read.
FYI it is working fine with other screen readers such as JAWS and NVDA.

Please launch the URL below to reproduce the issue.
There is table with prices in the page.
When using the table shortcuts, the headers of the table are not read.

After extracting the table without CSS and icons (cf attached file), the table is read correctly with the headings.

When testing in the page, we noticed that font icons are read even if hidden with aria-hidden attribute (VoiceOver announces them as “group …”).
This is not happening while testing in the extracted table as there is no CSS.
Could this be the cause of the issue for the table headings?
Also, how can we hide meaningless icons for VoiceOver users when aria-hidden is not taken into account? Will it be fixed in a next version?

Testing environment
We are testing on MAC OS 10.9.5 with Safari browser and VoiceOver.

Best regards

URL for test page

https://pcm.uat01.amadeus.com/plnext/demo/Override.action?SO_SITE_RUI_AX_CAL_ENABLED=TRUE&TRIP_FLOW=YES&BOOKING_FLOW=REVENUE&B_LOCATION_1=KEF&B_LOCATION_2=LON&B_DATE_1=201608110000&B_ANY_TIME_1=TRUE&E_LOCATION_1=LON&E_LOCATION_2=KEF&B_DATE_2=201608220000&B_ANY_TIME_2=TRUE&TRIP_TYPE=R&EXTERNAL_ID=BOOKING&PRICING_TYPE=O&EMBEDDED_TRANSACTION=FlexPricerAvailability&DISPLAY_TYPE=1&ARRANGE_BY=D&SO_SITE_MATRIX_CALENDAR=TRUE&REFRESH=0&COMMERCIAL_FARE_FAMILY_1=OWCOACHEU&COMMERCIAL_FARE_FAMILY_2=OWCOMFVIA&COMMERCIAL_FARE_FAMILY_3=OWCOMFVIA&DATE_RANGE_VALUE_1=3&DATE_RANGE_VALUE_2=3&DATE_RANGE_QUALIFIER_1=C&DATE_RANGE_QUALIFIER_2=C&TRAVELLER_TYPE_1=ADT&TRAVELLER_TYPE_2=&TRAVELLER_TYPE_3=&TRAVELLER_TYPE_4=&TRAVELLER_TYPE_5=&TRAVELLER_TYPE_6=&TRAVELLER_TYPE_7=&TRAVELLER_TYPE_8=&TRAVELLER_TYPE_9=&HAS_INFANT_1=FALSE&HAS_INFANT_2=FALSE&HAS_INFANT_3=FALSE&HAS_INFANT_4=FALSE&HAS_INFANT_5=FALSE&HAS_INFANT_6=FALSE&HAS_INFANT_7=FALSE&HAS_INFANT_8=FALSE&HAS_INFANT_9=FALSE&SO_SITE_EXT_PSPURL=&SO_SITE_EXT_MerchantID=FI&SO_GL=&SO_SITE_OFFICE_ID=KEFFI08AA&SO_SITE_QUEUE_OFFICE_ID=KEFFI08AA&SO_SITE_QUEUE_CATEGORY=0C0&SO_SITE_MOP_EXT=TRUE&SO_SITE_FOP_MODE=SITE&SITE=5APDDEMO&LANGUAGE=GB&SO_SITE_APIV2_SERVER_USER_ID=GUEST&SO_SITE_APIV2_SERVER=194.156.170.78&SO_SITE_APIV2_SERVER_PWD=TAZ&SO_SITE_CORPORATE_ID=OCGPDT&SO_SITE_SI_SAP=1ASIXJCP&SO_SITE_SI_SERVER_PORT=18033&SO_SITE_SI_SERVER_IP=193.23.185.67&SO_SITE_SI_USER=UNSET&SO_SITE_SI_PASSWORD=UNSET&SO_SITE_SI_1AXML_FROM=SEP_JCP&SO_SITE_FQ_INTERFACE_ACTIVE=FALSE&SO_SITE_HOST_TRACE_ACTIVE=TRUE&SO_SITE_FP_TRACES_ON=FALSE
Comment 1 Radar WebKit Bug Importer 2016-06-13 10:28:02 PDT
<rdar://problem/26771065>
Comment 2 chris fleizach 2016-06-13 12:43:17 PDT
Created attachment 281188 [details]
Header row is zero sized
Comment 3 chris fleizach 2016-06-13 12:45:11 PDT
So it appears that the table headings not read when adorned with all the CSS is because they are essentially hidden, so they're skipped in the inclusion of the headers the table includes.

However, the cell is able to see these headers, so I think ostensibly VoiceOver could fix this bug, or you can try to make sure that the header row is not zero sized

Where should I look for the aria-hidden font icons? Not sure what you're referring to

Thanks
Comment 4 helene.vinh 2016-06-14 02:52:37 PDT
Hello Chris,

Thanks for your quick answer.

Actually the headers of the table are not hidden but placed off the screen by the following CSS:

.plnext-sr-only {
    position: fixed;
    left: -999999px;
}

It is a technique that we use to add content for screen readers only, like additional instructions for example.
I am not sure what else we can do about it. How much time do you think it will take VoiceOver team to fix it?

For the font icons, you can try the top left calendar icon for example:
<i aria-hidden="true" class="icon-calendar calendarmatrix-calendar-icon" title="calendar"></i>

Also on each price cell, there is a tick icon which is displayed when the cell is selected:
<i aria-hidden="true" class="icon-ok-sign calendarmatrix-sign-ok-icon" title="Selection"></i>
Those are announced as "group ..."

Thanks & regards
Comment 5 chris fleizach 2016-06-14 11:16:49 PDT
(In reply to comment #4)
> Hello Chris,
> 
> Thanks for your quick answer.
> 
> Actually the headers of the table are not hidden but placed off the screen
> by the following CSS:
> 
> .plnext-sr-only {
>     position: fixed;
>     left: -999999px;
> }
> 
> It is a technique that we use to add content for screen readers only, like
> additional instructions for example.
> I am not sure what else we can do about it. How much time do you think it
> will take VoiceOver team to fix it?
> 
> For the font icons, you can try the top left calendar icon for example:
> <i aria-hidden="true" class="icon-calendar calendarmatrix-calendar-icon"
> title="calendar"></i>
> 
> Also on each price cell, there is a tick icon which is displayed when the
> cell is selected:
> <i aria-hidden="true" class="icon-ok-sign calendarmatrix-sign-ok-icon"
> title="Selection"></i>
> Those are announced as "group ..."

On macOS Sierra (10.12) and/or the latest safari technology preview, I don't see this problem with the css::before content. It appears properly hidden and I don't hear VoiceOver announce a different group.

A workaround idea would be to do something like

<span aria-hidden="true"><i ...></I></span>

So that the aria-hidden in on a higher level object


> 
> Thanks & regards
Comment 6 helene.vinh 2016-06-20 02:58:59 PDT
Hello,

Thanks for your input.
So with the latest version of MacOS, do you know if it is still relevant to add aria-hidden to font icons?

For the initial issue about the headers, could you confirm it will be fixed by VoiceOver?

Thanks & regards
Comment 7 chris fleizach 2016-06-20 13:30:59 PDT
(In reply to comment #6)
> Hello,
> 
> Thanks for your input.
> So with the latest version of MacOS, do you know if it is still relevant to
> add aria-hidden to font icons?

You mean latest released version? Or macOS Sierra? With macOS Sierra, no change is needed, it seems to work as expected

> 
> For the initial issue about the headers, could you confirm it will be fixed
> by VoiceOver?

I'm still looking into this. I'm sure there is a work around, but it should also (probably) be fixed in WebKit. If during investigation there is a workaround I will post here

> 
> Thanks & regards
Comment 8 helene.vinh 2016-07-20 05:35:04 PDT
Hello,

Is there any update for the webkit fix/ or workaround?

Thanks & regards
Comment 9 chris fleizach 2016-07-20 12:24:59 PDT
No update. Have you tested on Sierra yet?
Comment 10 helene.vinh 2016-07-25 06:55:01 PDT
Hello,

No, I don't have access to test with Sierra

Regards
Comment 11 helene.vinh 2016-08-18 08:56:02 PDT
Hello,

Any update on this issue please?

Thank you
Comment 12 helene.vinh 2016-11-29 05:35:10 PST
Hello,

Any update on this issue please?

Thank you
Comment 13 chris fleizach 2016-11-29 09:00:07 PST
have you tested with the latest macOS now that it's been released?
Comment 14 helene.vinh 2016-12-15 02:24:29 PST
Hello,

Yes we tested on Sierra and the issue is still occuring.
Could you please advise?

Thanks
Comment 15 chris fleizach 2016-12-15 18:14:54 PST
(In reply to comment #14)
> Hello,
> 
> Yes we tested on Sierra and the issue is still occuring.
> Could you please advise?
> 
> Thanks

Don't use  position:fixed; left:-99999px;  on the <th> tag

when I remove that the table header becomes accessible.
Comment 16 helene.vinh 2016-12-16 07:37:06 PST
Hello,

We use this CSS to make the headers invisible to sighted users.
An alternative that we could do is use:

position:fixed; opacity:0;

Will it work that way or is it really the position:fixed that is causing the issue? 

Best regards
Comment 17 chris fleizach 2016-12-16 08:46:34 PST
(In reply to comment #16)
> Hello,
> 
> We use this CSS to make the headers invisible to sighted users.
> An alternative that we could do is use:
> 
> position:fixed; opacity:0;
> 

Yes it does look like opacity:0 will fix the problem... thanks

> Will it work that way or is it really the position:fixed that is causing the
> issue? 
> 
> Best regards
Comment 18 helene.vinh 2016-12-16 09:12:01 PST
Actually, this might be an acceptable solution for desktop but could still cause issues on tablet.
The text will still be displayed on the page (even invisible) and could be read by VoiceOver when navigating on it with touch.

Is there any way that it will work by putting a smaller left value but still displaying the text outside of the screen?

Best regards
Comment 19 chris fleizach 2016-12-16 09:16:45 PST
(In reply to comment #18)
> Actually, this might be an acceptable solution for desktop but could still
> cause issues on tablet.
> The text will still be displayed on the page (even invisible) and could be
> read by VoiceOver when navigating on it with touch.
> 
> Is there any way that it will work by putting a smaller left value but still
> displaying the text outside of the screen?
> 
> Best regards

Not sure, feel free to play around with values. My guess is no, but I could be wrong
Comment 20 chris fleizach 2016-12-16 09:43:56 PST
Created attachment 297319 [details]
patch
Comment 21 WebKit Commit Bot 2016-12-16 09:46:20 PST
Attachment 297319 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/AccessibilityTableColumn.cpp:172:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 22 Darin Adler 2016-12-22 09:44:28 PST
Comment on attachment 297319 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityTableColumn.cpp:151
> +                    if (parentElement->hasTagName(trTag) && testCol < static_cast<int>(parentElement->children()->length()))

Please use countChildNodes() instead of children()->length() in C++ code.

> Source/WebCore/accessibility/AccessibilityTableColumn.cpp:152
> +                        testCellElement = parentElement->children()->item(testCol);

Please use traverseToChildAt(i) instead of children()->item(i) in C++ code.

> Source/WebCore/accessibility/AccessibilityTableColumn.cpp:175
> +    AccessibilityObject* cellObject = axObjectCache()->getOrCreate(cell);
> +    if (cellObject && !cellObject->accessibilityIsIgnored())
> +        return cellObject;
> +        
> +    return nullptr;

WebKit coding style calls for early return. That would be reversed, like this:

    auto* cellObject = axObjectCache()->getOrCreate(cell);
    if (!cellObject || cellObject->accessibilityIsIgnored())
        return nullptr;

    return cellObject;
Comment 23 chris fleizach 2016-12-25 22:29:18 PST
Created attachment 297760 [details]
patch
Comment 24 WebKit Commit Bot 2016-12-25 22:31:48 PST
Attachment 297760 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/AccessibilityTableColumn.cpp:172:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 25 WebKit Commit Bot 2016-12-25 23:48:43 PST
Comment on attachment 297760 [details]
patch

Clearing flags on attachment: 297760

Committed r210150: <http://trac.webkit.org/changeset/210150>
Comment 26 WebKit Commit Bot 2016-12-25 23:48:50 PST
All reviewed patches have been landed.  Closing bug.
Comment 27 helene.vinh 2017-01-02 06:25:53 PST
Hello,

Thank you for the fix.
When will it be available? Do we get the patch by updating the Safari version?

Thanks & regards
Comment 28 helene.vinh 2017-05-02 08:38:28 PDT
Hello,

Could you please give an answer to my question above?

Also, will this fix be available on ipad? We are encountering the same issue on ipad version 10.3.1

Thanks & regards
Comment 29 chris fleizach 2017-05-02 09:54:11 PDT
Apple doesn't pre-announce when releases will be available.

The fix will come to iPad too