Closed Bug 1132140 Opened 9 years ago Closed 4 years ago

WARNING spewage for text-changed signals being invalid for MaiAtkType25

Categories

(Core :: Disability Access APIs, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: jdiggs, Unassigned)

Details

(Keywords: good-first-bug, Whiteboard: [lang=c++])

Attachments

(1 file)

Steps to reproduce:
1. Launch Firefox in a terminal in a *nix environment with a11y enabled.*
2. Load the attached test case and press the 'Load schedule' button

*Note: You may also need to register for accessible text-changed events (e.g. via Accerciser or a stand-alone pyatspi accessible-event listener).

Expected results: No error spewage

Actual results: A bunch of spewage a la:
(firefox:3467): GLib-GObject-WARNING **: gsignal.c:3410: signal name 'text-insert::system' is invalid for instance '0x7fe970a96150' of type 'MaiAtkType25'
(firefox:3467): GLib-GObject-WARNING **: gsignal.c:3410: signal name 'text-remove::system' is invalid for instance '0x7fe970a96150' of type 'MaiAtkType25'
So, we have a <span> with HyperTextAccessible of role PROGRESSMETER.  That means we don't expose the atk text interface because of AccessibleWrap.cpp:366.  But we fire a text change event because someone does span.firstChild = text node or similar.

So the question is should this span implement the accessible text interface and have text, or should it not do that and not fire text change events?
If it's exposed to us as a progress bar, as far as we're concerned it's a progress bar. And progress bars are not objects which should emit the accessible text interface (my opinion).

On a relatedish note, when I run one of the dojo slider demos as a test, I get:
(firefox:5305): GLib-GObject-WARNING **: gsignal.c:3410: signal name 'text_caret_moved' is invalid for instance '0x7f1dff9e7420' of type 'MaiAtkType27'
(In reply to Joanmarie Diggs from comment #2)
> If it's exposed to us as a progress bar, as far as we're concerned it's a
> progress bar. And progress bars are not objects which should emit the
> accessible text interface (my opinion).

that shouldn't be hard to do, but I think the below calls into question if its a good idea.  In that case not only is there a span with text content, but someone is moving the caret through that content.  So if we pretend like there is no text there and drop those events on the floor should you just say nothing as someone arros through the "progress meter"?  I really hope this happening is an author error and so UB, but it seems to me we shouldn't do extra work to make things worse for users.

> 
> On a relatedish note, when I run one of the dojo slider demos as a test, I
> get:
> (firefox:5305): GLib-GObject-WARNING **: gsignal.c:3410: signal name
> 'text_caret_moved' is invalid for instance '0x7f1dff9e7420' of type
> 'MaiAtkType27'

this is escentially the same issue.
More essentially the same issue can be found on http://files.paciellogroup.com/blogmisc/samples/aria/slider. In particular, complaints about text-remove::system, text-insert::system, text_selection_changed, and text_caret_moved for MaiAtkType27.

Since that test case is at the Paciello Group, my guess is that there's no author error there. And sliders dressed in ARIA are not text objects. I'm pretty sure due to the invalid nature of the signals, that they are never making it as far as an AT or end user. This is a good thing. If you want to keep spewing errors, go for it. The one thing I would ask that you do not do is implement AtkText on these pseudo widgets. While it would make the spewage go away, it would give Orca even more crap to examine and ultimately discard as bogus. And that's sucky for the user too.
(In reply to Joanmarie Diggs from comment #4)
> More essentially the same issue can be found on
> http://files.paciellogroup.com/blogmisc/samples/aria/slider. In particular,
> complaints about text-remove::system, text-insert::system,
> text_selection_changed, and text_caret_moved for MaiAtkType27.
> 
> Since that test case is at the Paciello Group, my guess is that there's no
> author error there. And sliders dressed in ARIA are not text objects. I'm
> pretty sure due to the invalid nature of the signals, that they are never
> making it as far as an AT or end user. This is a good thing. If you want to
> keep spewing errors, go for it. The one thing I would ask that you do not do
> is implement AtkText on these pseudo widgets. While it would make the
> spewage go away, it would give Orca even more crap to examine and ultimately
> discard as bogus. And that's sucky for the user too.

I was planning on either not firing the events, or making these things implement atk text.

tbh I think having them implement atk text kind of makes more sense after all they are text objects in that there is text on the screen.  However I guess if you really think that text shouldn't be exposed I  guess I'm willing to go the not firing events route.
Well, my test case was something I found... Somewhere.... And added it as a regression test. It could easily be author error. But I honestly don't think that's the case for the sliders at TPG. And I don't think those sliders at TPG should be emitting the events in question let alone claiming to implement accessible text.

On a vaguely related note: If you view http://archive.dojotoolkit.org/nightly/dojotoolkit/dijit/tests/test_Dialog.html and then click the "Show Dialog" button, you get GLib-GObject-WARNING **: gsignal.c:3410: signal name 'load_complete' is invalid for instance '0x7f7918c916f0' of type 'MaiAtkType139'

(Please don't implement AtkDocument if it's not a document. *grins*)
(In reply to Joanmarie Diggs from comment #6)
> Well, my test case was something I found... Somewhere.... And added it as a
> regression test. It could easily be author error. But I honestly don't think
> that's the case for the sliders at TPG. And I don't think those sliders at
> TPG should be emitting the events in question let alone claiming to
> implement accessible text.

I don't think I can spend the time to examine each test case in detail, so I'll just let you have your way until someone complains about things they miss because of it.

> On a vaguely related note: If you view
> http://archive.dojotoolkit.org/nightly/dojotoolkit/dijit/tests/test_Dialog.
> html and then click the "Show Dialog" button, you get GLib-GObject-WARNING
> **: gsignal.c:3410: signal name 'load_complete' is invalid for instance
> '0x7f7918c916f0' of type 'MaiAtkType139'
> 
> (Please don't implement AtkDocument if it's not a document. *grins*)

I suspect that's an element with role=document or role=dialog I think "native" dialogs in firefox  end up implementing the document interface, but I'm not sure off hand.
surkov this should make a good first bug if you can find someone to help people with it someone just needs to add a check for IsTextRole somewhere in logic dispatching text change events
let's put good-first-bug status, we'll figure out mentoring person when somebody picks it up
Whiteboard: [good first bug][lang=c++]
Keywords: good-first-bug
Whiteboard: [good first bug][lang=c++] → [lang=c++]

Is this issue still open?

Running on the latest build changeset: 613278:ab8e3128766e I can't replicate this on on CentOS 8 using a default ./mach build && ./mach run

I think we can probably close this out.

Closed WORKSFORME based on comment #11.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: