search

phetsims / balloons-and-static-electricity

"Balloons and Static Electricity" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/balloons-and-static-electricity
GNU General Public License v3.0
6 stars 10 forks source link

Safari says "empty movable" in its hint for interaction with a grabbed balloon #525

Open terracoda opened 3 years ago

terracoda commented 3 years ago

In https://github.com/phetsims/balloons-and-static-electricity/issues/524#issuecomment-896217219

it was noted that Voice over says "You are on an empty movable" when the Yellow Balloon is grabbed.

@jessegreenberg, can we check to see if this related to the issue that is investigating why VO is not finding the accessible name on the grab button?

If it is related, maybe we can force the accessible name when the balloon is in it's grabbed state?

I can test if Firefox and Chrome to see if the Accessible name is coming through in those browsers.

jessegreenberg commented 3 years ago

Correct @terracoda, this seems very related to https://github.com/phetsims/balloons-and-static-electricity/issues/519, and after that issue we confirmed that when VoiceOver lands on the "Grab"button it says

"Grab Yellow Balloon, button"

And when it lands on the "draggable" it says

"Yellow Balloon, movable"

The output

"You are currently on an empty movable"

only happens when "Speak instructions for using the item in the VoiceOver cursor" option is enabled.

jessegreenberg commented 3 years ago

Here is the HTML for the "grabbed" object image

It has an aria-label (which helped force the name in #519). It also has inner content. Maybe it is "empty" because there is no HTML under the element with the application role?

jessegreenberg commented 3 years ago

I tried moving the role="application" to the parent div so that it doesn't say "empty", thinking that perhaps having role="application" on an element with no HTML children would be "empty" to VoiceOver. But it did not fix it.

jessegreenberg commented 3 years ago

I was sort of correct. If I change the markup to this, "empty" goes away and it just says "movable".

<div aria-roledescription="movable" role="application" tabindex="0">
  <p>Yellow Balloon</p>
</div>

Here is the patch for GrabDragInteraction

```patch Index: js/accessibility/GrabDragInteraction.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- js/accessibility/GrabDragInteraction.js (revision 7a6b0480e7b81dc39ec48dc369bd951f59df9523) +++ js/accessibility/GrabDragInteraction.js (date 1628625954109) @@ -225,7 +225,8 @@ // @private this.draggableAccessibleName = options.objectToGrabString; - options.draggableOptions.innerContent = this.draggableAccessibleName; + // options.draggableOptions.innerContent = this.draggableAccessibleName; + options.draggableOptions.innerContent = null; options.draggableOptions.ariaLabel = this.draggableAccessibleName; assert && assert( !options.grabbableOptions.accessibleName, 'GrabDragInteraction sets its own accessible name, see objectToGrabString' ); @@ -304,6 +305,11 @@ } } ); + this.extraLabelNode = new Node( { + tagName: 'p', + innerContent: this.draggableAccessibleName + } ); + // @private - wrap the optional onRelease in logic that is needed for the core type. this.onRelease = () => { options.onRelease && options.onRelease(); @@ -544,8 +550,13 @@ this.node.removeAriaDescribedbyAssociation( this.descriptionAssociationObject ); } + if ( this.node.hasChild( this.extraLabelNode ) ) { + this.node.removeChild( this.extraLabelNode ); + } + this.baseInteractionUpdate( this.grabbableOptions, this.listenersForDragState, this.listenersForGrabState ); + // callback on completion this.onGrabbable(); } @@ -572,6 +583,8 @@ // turn this into a draggable in the node this.baseInteractionUpdate( this.draggableOptions, this.listenersForGrabState, this.listenersForDragState ); + this.node.addChild( this.extraLabelNode ); + // callback on completion this.onDraggable(); } ```

This is not a VoiceOver bug, the element with role=application is in fact "empty" of children. Just silly VO decided to tell the user that in my opinion.

Next questions:

jessegreenberg commented 3 years ago

I just tried putting the role and role-description on a parent of the focusable but it said "You are on an empty group" instead. image

terracoda commented 3 years ago

This is interesting to say the least. It must have something to do with flow content and screen readers. Paragraph elements are expected to have content, whereas div-elements may or may not.

terracoda commented 3 years ago

Nope, I think this has to do with the difference between flow content and phrasing content http://www.html-5.com/definitions/flow-content.html

terracoda commented 3 years ago

Anyways, you are right this is not a bug, and I agree that Voice Over is just not understanding that there is content in the div. Using an HTML element that is meant for phrasing content seems like a good idea, but I am not sure it is critical for this release if it causes other serious problems.

I want to be clear on the what you are asking about the HTML. Are you saying we need to choose between:

Code A sample:

<div aria-roledescription="movable" role="application" tabindex="0">
  Yellow Balloon
</div>

or code B sample:

<div aria-roledescription="movable" role="application" tabindex="0">
  <p>Yellow Balloon</p>
</div>

And that code B sample causes some big highligting issues?

terracoda commented 3 years ago

With Code A sample, Voice Over thinks the div is empty, and in the code B sample, Voice Over recognizes the content?

Does VO say, "You are currently on a Yellow Balloon movable" in code B sample?

jessegreenberg commented 3 years ago

In https://github.com/phetsims/balloons-and-static-electricity/issues/525#issuecomment-896922768 Code A sample is what we have now and code B sample is what makes "empty" go away. For the HTML I was asking if there is different HTML structure you want to try that might be better?

Does VO say, "You are currently on a Yellow Balloon movable" in code B sample?

With sample B VoiceOver says "You are currently on a movable."

terracoda commented 3 years ago

Ok, thanks @jessegreenberg. Nesting a p-element inside the div-element with application role is likely better code than not using a p-tag and worth investigating, but maybe this can wait because it causes other issues with highlights.

This is a Voice Over hint, not our design. The improvement from "empty movable" to "movable" alone, is definitely better, but not worth the impact risk.

I am going to mark this as deferred for now. Maybe we can investigate this further in Friction's grab button, as well?

terracoda commented 3 years ago

This issue is not related to #519. I think it has to do with some subtle differences between elements meant for flow content and elements meant for phrasing content. Both P-elements and Div-elements are meant for flow content, but of the two, I think only P-elements are meant for phrasing content.

jessegreenberg commented 3 years ago

Discussed with @terracoda - we agreed with the above comments and we will not work on this now. When we work on friction which also has the grab drag interaction we can revisit this. This sim will likely get released again after interactive highlights are improved and we can republish BASE after that is done.

jessegreenberg commented 3 years ago

Removing my assignment until we revisit this in the next use case.