mate-desktop / mate-applets

Applets for use with the MATE panel
http://www.mate-desktop.org
GNU General Public License v2.0
79 stars 67 forks source link

timer: fix in-process ATK warning #655

Closed lukefromdc closed 1 year ago

lukefromdc commented 1 year ago

Fix atk_object_set_description: assertion 'description != NULL' failed warning when timer runs down. Looks like a race condition is present here. Ideally someone with a working orca install (errors out here) would test this, but the warning indicates that we would not be setting anything whenatk_object_set_description gets called with an empty description anyway.

Alternately, we could set that description to "finished timer" but Orca should already speak the label text, which shows up in Accerciser

raveit65 commented 1 year ago

I can confirm that PR fixes the warning when build in-process and running in wayfire session. No idea if this is a valid fix.

lukefromdc commented 1 year ago

Should we hold this pending an A/B/A test with a working Orca install and knowing what's supposed to be spoken there? I could not figure out with Accerciser what's supposed to be going on

raveit65 commented 1 year ago

I think @cwendling can help as member of hypra.fr It' only a warning and we don't have any rush, ihmo.

lukefromdc commented 1 year ago

At least Orca is supposed to work on wayland as well as x11, though git master Orca is needed as of the last report on the GNOME wiki:

https://wiki.gnome.org/Accessibility/Wayland

Thus by the time distrops are shipping a mate-wayland session, Orca should work with it mostly the same as it does now in x11.

lukefromdc commented 1 year ago

What I can reasonably figure about this is the warning is aboutt setting an empty description, which would never be useful. Whether this is happening because of a race condition (e.g trying to read the tooltip text while it is changing), because we should not read (and Orca should not speak) the tooltip text when the tooltip is not showing, or something else I do not know.]

Using the tooltip text when we are setting a description unconditionally strikes me as odd. If we want Orca to always have access to that, the text should go to ATK and the tooltip in parallel and not in series. I do NOT know if that is intended though.

This would depend on whether Orca picks up the dialog or notification and speaks it or not, and with Orca broken here (throws a python error), I cannot test that.

lukefromdc commented 1 year ago

I can test that and find out.

lukefromdc commented 1 year ago

Found the problem: tooltip is a gchar not the tooltip itself. In the if (!applet->active) tooltip is set to NULL and is not touched again until we attempt to read it into atk_object_set_description().

https://docs.gtk.org/atk/method.Object.set_description.html Using NULL for description is not allowed, for an empty value we are supposed to use ""instead.

This is the same ATK object that intimer_applet_fill is initially given this description: "Start a timer and receive a notification when it is finished"

In both cases atk_obj = gtk_widget_get_accessible (GTK_WIDGET (applet->applet)); so this is the SAME atk object, and we are emptying the text when the applet is inactive.The update I just force-pushed retains this behavior (must have been some reason for it such as keeping spam off Orca) but does so in the manner called for in the ATK documentation

cwendling commented 1 year ago

Sorry to be late here, but actually we could just remove this set_description() call, it doesn't do anything useful. And IIUC is was meant to be set to the tooltip text, which is set later on in this function, but doing so gives a terrible screen reader experience, so at least some more care would have to be taken. No description is fine here, and removing the line is just as good (but this change is not a problem either).