After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 675341 - "Invisible" text is not invisible to ATs like Orca
"Invisible" text is not invisible to ATs like Orca
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.4.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2012-05-02 23:32 UTC by Joanmarie Diggs (IRC: joanie)
Modified: 2012-06-05 16:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Tentative patch (1.37 KB, patch)
2012-05-03 19:12 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
none Details | Review
Add "hidden" style class (1.65 KB, patch)
2012-06-04 16:29 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
none Details | Review
Do not expose label text is hidden style is used (1.33 KB, patch)
2012-06-04 16:30 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
reviewed Details | Review
Add "hidden" style class (1.62 KB, patch)
2012-06-05 07:36 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
committed Details | Review
Do not expose label text if hidden style is used (929 bytes, patch)
2012-06-05 07:37 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
committed Details | Review

Description Joanmarie Diggs (IRC: joanie) 2012-05-02 23:32:50 UTC
According to the comment found here [1]:

/* text is intentionally non-blank otherwise the height is not the same as for
 * infoMessage and errorMessageLabel - but it is still invisible because
 * gnome-shell.css sets the color to be transparent
 */
this._nullMessageLabel = new St.Label({ style_class: 'prompt-dialog-null-label',
                                        text: 'abc'});

But Orca is receiving AT-SPI2 events which suggest the text is showing. Here's the output of a simple pyatspi listener script:

  Widgets claiming STATE_SHOWING
  [label | Authentication Required]
  [label | To change time or date settings, you need to authenticate.]
  [label | Joanmarie Diggs]
  [label | Password: ]
  [label | abc]
  [push button | Cancel]
  [push button | Authenticate]

If it is indeed necessary to have the 'abc' text present but hidden, it would be extremely helpful to prevent it from announcing to AT-SPI2 that it is showing. Alternatively, some means of alerting Orca to the fact that it's functionally hidden would be appreciated.

As an aside, Orca doesn't present all of the labels in this dialog *yet*. I was working on that, and couldn't figure out why Orca kept saying 'abc'. ;)

[1] http://git.gnome.org/browse/gnome-shell/tree/js/ui/polkitAuthenticationAgent.js#n164
Comment 1 Alejandro Piñeiro Iglesias (IRC: infapi00) 2012-05-03 19:12:18 UTC
Created attachment 213402 [details] [review]
Tentative patch

Well, the first solution that I found is just avoiding to expose the text if "prompt-dialog-null-label" style class is used. Taking into account that name, and that comment, it seems to be used to represent text that shouldn't be exposed.

Joanmarie was gently enough to test this patch, and works.

Having said so ... the problem with this patch is that it is a hack. After all I'm hardcoding a style class name on the patch. Anyway, IMHO, this seems somewhat a collateral effect of the original solution used to ensure the same height of two different labels. Although I didn't take too much time to review all the options, the solution of setting a fake text that shouldn't be showed sound somewhat hacky to me.

Final note: take into account that this is not the first time that the accessibility support take literal names to know what it is going on. For the case of selected and checked items, pseudo class "selected" and "checked" are checked, for simplicity sake.
Comment 2 Joseph Scheuhammer 2012-05-08 19:44:42 UTC
Some related thoughts:  some time in the past, when web content was styled so as to not be seen, and ATs looked directly at the DOM, they missed the fact that the content wasn't actually rendered, and presented it by mistake.  Sample markup follows:
<div style="visibility: hidden;">This text is not rendered</div>.

That's because one can't tell by looking only at the DOM whether something is perceivable or not.  One also has to take into account the element's computed style.

ARIA introduced the aria-hidden attribute to help mitigate this problem.  The idea is if an author styles content so it is not rendered, they must also add aria-hidden="true":
<div style="visibility: hidden;" aria-hidden="true">This text is not rendered</div>.

That way, if an AT looks directly at the DOM, they can check for the aria-hidden attribute and react as appropriate.

Also, browsers expose aria-hidden via AT-SPI.  With FF, there is an accessible object corresponding to the <div>, but it has an AT-SPI object attribute of 'hidden' set to 'true'.  WebKit and IE go further: when they encounter an aria-hidden element, they do not create an accessible in the a11y tree.  (Nit: to be complete, no browser provides an accessible for something whose style is 'visibility:hidden' Ditto for the 'display:none' style).

But that's all with markup and browsers -- how might this be fixed in the present case?

1. CSS visibility:hidden does not render the content, but does render empty space that is the size that the content would occupy.  This is the preferred way of hiding text on the web while still occupying the same space.  That is, don't use transparency for this.  But, I don't know if the St toolkit's CSS parser/renderer is as sophisticated as gecko or webkit here -- can it use visibility:hidden?

2. Even so, if the intent is to hide the content, then the content should somehow be marked as hidden at the AT-SPI level; either by not exposing any accessible, or by marking its hidden attribute as true.  In other words, API's hack is not really that much of a hack after all.  The only hack-y bit is how specific the patch is with respect to the 'prompt-dialog-null-label' class.  If it could somehow generalize on a rendered style (visibility: hidden and/or display:none), that would be better.
Comment 3 Jasper St. Pierre (not reading bugmail) 2012-05-08 19:47:28 UTC
(In reply to comment #2)
> 1. CSS visibility:hidden does not render the content, but does render empty
> space that is the size that the content would occupy.  This is the preferred
> way of hiding text on the web while still occupying the same space.  That is,
> don't use transparency for this.  But, I don't know if the St toolkit's CSS
> parser/renderer is as sophisticated as gecko or webkit here -- can it use
> visibility:hidden?

It would be really simple to add.
Comment 4 Alejandro Piñeiro Iglesias (IRC: infapi00) 2012-05-09 11:34:05 UTC
Joseph, thanks for the explanation.

(In reply to comment #2)
> 1. CSS visibility:hidden does not render the content, but does render empty
> space that is the size that the content would occupy.  This is the preferred
> way of hiding text on the web while still occupying the same space.  That is,
> don't use transparency for this.  But, I don't know if the St toolkit's CSS
> parser/renderer is as sophisticated as gecko or webkit here -- can it use
> visibility:hidden?
> 
> 2. Even so, if the intent is to hide the content, then the content should
> somehow be marked as hidden at the AT-SPI level; either by not exposing any
> accessible, or by marking its hidden attribute as true.  In other words, API's
> hack is not really that much of a hack after all.  The only hack-y bit is how
> specific the patch is with respect to the 'prompt-dialog-null-label' class.  If
> it could somehow generalize on a rendered style (visibility: hidden and/or
> display:none), that would be better.

Yes, although I didn't explain it really well, the real hacky nature of the patch was how to detect that the element is hidden.

I also forgot to mention that the idea of the patch is showing that there is a way to solve it, but it would be good to find a proper general solution. Good that you understood that without me saying.

If 1. is implemented, I guess that somehow the StLabel could ask for that CSS property. That would replace my 'prompt-dialog-null-label' check. Then I could do as my patch does, not exposing the text in that case, or setting the state to hidden (right now, not exposing ATK_STATE_SHOWING).
Comment 5 Alejandro Piñeiro Iglesias (IRC: infapi00) 2012-05-09 11:35:58 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > 1. CSS visibility:hidden does not render the content, but does render empty
> > space that is the size that the content would occupy.  This is the preferred
> > way of hiding text on the web while still occupying the same space.  That is,
> > don't use transparency for this.  But, I don't know if the St toolkit's CSS
> > parser/renderer is as sophisticated as gecko or webkit here -- can it use
> > visibility:hidden?
> 
> It would be really simple to add.

Ok, then lets maintain my patch on "tentative" status. If finally this CSS visibility:hidden is implemented I could just update my patch. If not we can upgrade my patch status from "tentative" to "best option right now" and apply it.

Thanks.
Comment 6 Joseph Scheuhammer 2012-05-09 15:12:40 UTC
(
> Ok, then lets maintain my patch on "tentative" status. If finally this CSS
> visibility:hidden is implemented I could just update my patch.
> ...

Ideally:
1. implement support for the CSS 'visibility' selector and its values { visible | hidden | collapsed } -- see http://www.w3.org/TR/CSS2/visufx.html#visibility

2. implement support for the 'display' selector and its values (too many to list here, but the most important with respect to this bug is 'none') -- see http://www.w3.org/TR/CSS2/visuren.html#propdef-display

3. implement the getComputedStyle() method -- see http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-OverrideAndComputed.  This is what the patch would use to get the styling info for the St widget, and from there determine its visibility/display, and what to publish to the a11y layer.

That sounds a lot like implementing a browser, and might be overkill.  Note the "Ideally" qualifier.
Comment 7 Alejandro Piñeiro Iglesias (IRC: infapi00) 2012-06-04 16:15:07 UTC
(In reply to comment #6)

> That sounds a lot like implementing a browser, and might be overkill.  Note the
> "Ideally" qualifier.

In relation with that, I have been talking with Jasper on IRC. I proposed to use something similar to my previous patch meanwhile all the CSS stuff is implemented. We agreed that check for something called "prompt-dialog-null-label" doesn't seems too formal, so he proposed to add a style class called hidden.
Comment 8 Alejandro Piñeiro Iglesias (IRC: infapi00) 2012-06-04 16:29:16 UTC
Created attachment 215562 [details] [review]
Add "hidden" style class

This patch adds a hidden style class, and uses it on polkitAuthenticationAgent.

As that label uses both styles, I think that it is useless (and probably error-prone) to keep the invisible color on prompt-dialog-null-label, so I removed that from that style class.
Comment 9 Alejandro Piñeiro Iglesias (IRC: infapi00) 2012-06-04 16:30:19 UTC
Created attachment 215563 [details] [review]
Do not expose label text is hidden style is used

On StLabelAccessible is checks the style of the base object. If it includes 'hidden' the text is not exposed.
Comment 10 Jasper St. Pierre (not reading bugmail) 2012-06-05 04:37:07 UTC
Review of attachment 215563 [details] [review]:

::: src/st/st-label.c
@@ +481,3 @@
+{
+  /* See bug BGO#675341: 'hidden' style is used similar way that
+     hidden tag on html. In that case we don't expose the text */

HTML doesn't have a hidden tag. I don't think this comment is necessary.

@@ +504,1 @@
       if (actor == NULL) /* State is defunct */

if (actor == NULL || st_widget_has_style_class_name (ST_WIDGET (label), "hidden"))
Comment 11 Alejandro Piñeiro Iglesias (IRC: infapi00) 2012-06-05 07:36:19 UTC
Created attachment 215612 [details] [review]
Add "hidden" style class

Removed reference to "hidden" tag on the commit message (sorry, I was talking about the attribute value for input fields)
Comment 12 Alejandro Piñeiro Iglesias (IRC: infapi00) 2012-06-05 07:37:13 UTC
Created attachment 215613 [details] [review]
Do not expose label text if hidden style is used

Updated patch after review on comment 10
Comment 13 Jasper St. Pierre (not reading bugmail) 2012-06-05 13:30:32 UTC
Review of attachment 215612 [details] [review]:

Looks fine.
Comment 14 Jasper St. Pierre (not reading bugmail) 2012-06-05 13:30:40 UTC
Review of attachment 215613 [details] [review]:

Sure as well.