Bug 155447 - Add accessible object attributes to LO spellcheck dialog components for reliable identification by ATs
Summary: Add accessible object attributes to LO spellcheck dialog components for relia...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium enhancement
Assignee: Michael Weghorn
URL:
Whiteboard: target:25.2.0
Keywords: accessibility
Depends on:
Blocks: a11y, Accessibility
  Show dependency treegraph
 
Reported: 2023-05-23 11:05 UTC by Joanmarie Diggs
Modified: 2024-11-08 15:57 UTC (History)
1 user (show)

See Also:
Crash report or crash signature:


Attachments
Screenshot of "id" object attribute used in Thunderbird's spell check dialog (132.05 KB, image/png)
2024-08-07 08:50 UTC, Michael Weghorn
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joanmarie Diggs 2023-05-23 11:05:44 UTC
Description:
Orca has customized presentation for application spell-check dialogs:
* Announces the misspelled word, along with its spelling
* Announces the surrounding text for context
* Announces the current suggestion from the suggestion list, along with its spelling

Because there is no spellcheck-specific accessibility API, Orca has fragile heuristics to identify:
* Whether or not the current window/dialog is the spellcheck dialog
* The widget containing the misspelled word
* The suggestion list

Changes in the LO spellcheck dialog can -- and do -- break those heuristics. :( And there is also the danger of false positives....

Under the circumstances, here is my proposal to make things a bit better, both for Linux and for Windows: Add accessible object attributes to the LO spellcheck dialog components so that ATs could identify them. For instance:

* On the dialog: is-spellcheck-dialog:true
* On the suggestions list: is-suggestions-list:true
* On the widget displaying the error: contains-error:true

Mind you, I don't care what the object attributes are as long as they are predictable. :) And maybe after LO does this, we can convince Thunderbird and Evolution and Gedit/Pluma and ... to use whatever those attributes happen to be.

Because IA2 also has object attributes, NVDA, JAWS, etc. should be able to also benefit from this solution.


Steps to Reproduce:
1. Create a document with misspelled words
2. Launch Orca (version 44.0 or earlier)
3. Press F7 for the spellcheck dialog


Actual Results:
Orca fails to announce the misspelled word, its spelling, the text surrounding the error, the suggested correction, etc.


Expected Results:
Orca would announce the misspelled word, its spelling, the text surrounding the error, the suggested correction, etc.



Reproducible: Always


User Profile Reset: No

Additional Info:
Apparently the spelling dialog has changed:

* The role of the window firing the window activate event
* The role of the error-message widget
* The role of the suggestions list widget
* The events fired when a suggestion has been accepted or ignored and a new error presented
* Maybe other things?
Comment 1 Michael Weghorn 2023-05-26 03:12:52 UTC
Sounds like a reasonable request to look further into.
Comment 2 Michael Weghorn 2023-10-19 09:11:24 UTC
(In reply to Joanmarie Diggs from comment #0)
> Under the circumstances, here is my proposal to make things a bit better,
> both for Linux and for Windows: Add accessible object attributes to the LO
> spellcheck dialog components so that ATs could identify them. For instance:

From what I can see, neither Gtk 4 nor Qt currently support setting object attributes, but always return an empty set (Qt, [1]) or only report a very limited set of predefined ones (Gtk 4, [2]).

I'm hesitant to implement something that only works for gtk3, so I tend to think that implementing support in Gtk 4 and Qt would be a prerequisite in case of depending on those attributes being present.

Maybe, an alternative might be to use the "AccessibleId" property of the Accessible interface to identify it, and set that to the "id" attribute as specified in the .ui file (cui/uiconfig/ui/spellingdialog.ui), currently (but that could be adapted as needed):

* "SpellingDialog" for the dialog
* "suggestionslb" for the suggestions list
* "sentence" for the widget displaying the text/error

I haven't looked into that in detail yet, but from a first glance, it looks like that should be doable with Gtk 3 and Qt (which uses the "full hierarchy path" for the AccessibleId, so would probably have something like "SpellingDialog.<parent1>.<parent2>.<parent3>.suggestionslb", but that could be handled on Orca side.

Gtk 4 currently always reports an empty AccessibleId, but I could come up with a MR suggesting to use the "buildable_id", which to me seems to fit with the idea of what AccessibleId is there for.

>     <!--
>         AccessibleId: application-specific identifier for the current object.
> 
>         You can use this to give a special id to an object to use in tests, for example,
>         "my_widget".  Note that there is no way to directly find an object by its id; your
>         test program may have to recursively get the children to find a specific id.  This
>         is because accessible objects can be created dynamically, and they do not always
>         correspond to a static view of an application's data.
>     -->
>     <property name="AccessibleId" type="s" access="read"/>

For IAccessible2, that could still be mapped to an "accessible-id:<id>;" atttribute or somesuch if considered useful.

@Joanmarie: What do you think? Would that be helpful?


[1] https://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/accessible/linux/atspiadaptor.cpp?id=546208f0ff23819d216cbb5bf0b5daded79b454e#n1608
[2] https://gitlab.gnome.org/GNOME/gtk/-/blob/7325121c63e5ab54d6cea483f932b60224146061/gtk/a11y/gtkatspicontext.c#L524-544
[3]
Comment 3 QA Administrators 2024-05-06 03:15:09 UTC Comment hidden (obsolete)
Comment 4 QA Administrators 2024-06-06 03:17:29 UTC Comment hidden (obsolete)
Comment 5 Michael Weghorn 2024-06-06 12:12:46 UTC
Reopening.

@Joanmarie: Would settingh an AccessibleId for AT-SPI2 instead of setting an object attribute work, as suggested in comment 2?
Comment 6 Joanmarie Diggs 2024-06-06 15:24:58 UTC
(In reply to Michael Weghorn from comment #5)
> Reopening.
> 
> @Joanmarie: Would settingh an AccessibleId for AT-SPI2 instead of setting an
> object attribute work, as suggested in comment 2?

Maybe.... 

I don't know how other apps or toolkits are using an AccessibleId. Whatever solution is agreed upon, I'd like it to be something that everyone uses. But given this statement:

> You can use this to give a special id to an object to use in tests

It sounds like API specifically for tools/uses that are NOT screen readers.
Comment 7 Michael Weghorn 2024-06-11 15:05:37 UTC
(In reply to Joanmarie Diggs from comment #6)
> I don't know how other apps or toolkits are using an AccessibleId. Whatever
> solution is agreed upon, I'd like it to be something that everyone uses.

That sounds reasonable. I'd also prefer having a more widely used solution rather than something LO-specific.
Do you know what's the best way to find a consensus on something that would be used more widely?

> But
> given this statement:
> 
> > You can use this to give a special id to an object to use in tests
> 
> It sounds like API specifically for tools/uses that are NOT screen readers.

I don't  think this is set in stone, and the doc for the ATK equivalent is not restricted to testing either, so I think it makes sense to adjust the AT-SPI XML documentation as well. Pending MR:
https://gitlab.gnome.org/GNOME/at-spi2-core/-/merge_requests/161

From an implementation perspective, using the AccessibleId would IMHO have the advantage that it wouldn't require introducing new object attributes and there's already existing API to easily set that for ATK, GTK 3 and Qt at least.
Comment 8 Commit Notification 2024-07-17 04:25:40 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/5b9dec4caa727cd46ae9163812ecfc3142157cd1

tdf#155447 qt a11y: Don't report "Unknown" for unsupported props

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 9 Commit Notification 2024-07-17 04:25:43 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/c64b823b74cbd3063d6e87643bd68ea5343b2f54

tdf#155447 qt a11y: Report accessible ID

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 10 Commit Notification 2024-07-17 04:25:45 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/8495e77f4d49733f645cdacfb71729b91e5c4f30

tdf#155447 gtk3 a11y: Use GtkBuilder ID as accessible ID

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Michael Weghorn 2024-07-17 09:57:47 UTC
(In reply to Michael Weghorn from comment #7)
> > It sounds like API specifically for tools/uses that are NOT screen readers.
> 
> I don't  think this is set in stone, and the doc for the ATK equivalent is
> not restricted to testing either, so I think it makes sense to adjust the
> AT-SPI XML documentation as well. Pending MR:
> https://gitlab.gnome.org/GNOME/at-spi2-core/-/merge_requests/161
> 
> From an implementation perspective, using the AccessibleId would IMHO have
> the advantage that it wouldn't require introducing new object attributes and
> there's already existing API to easily set that for ATK, GTK 3 and Qt at
> least.

The at-spi2-core MR has been merged and with the commits from comments 8 to 10 in place, accessible IDs are now set for both, gtk3 and qt6.

"SpellingDialog" is the accessible ID for the dialog.
Orca MR demonstrating how this can be used:

https://gitlab.gnome.org/GNOME/orca/-/merge_requests/220

If considered a proper solution, this could be extended to also use the accessible ID for the individual components in the dialog.

Then: probably makes sense to agree on proper IDs and ideally add a test case to prevent unnoticed change.

GTK 4 issue suggesting to support accessible ID there as well:
https://gitlab.gnome.org/GNOME/gtk/-/issues/6870

@joanie: What do you think?
Comment 12 Michael Weghorn 2024-07-17 10:27:00 UTC
Related pending Qt change adding API to simplify setting an accessible ID for native Qt widgets (would primarily become relevant in case of welding, see tdf#130857):
https://codereview.qt-project.org/c/qt/qtbase/+/576691
Comment 13 Commit Notification 2024-07-18 04:53:06 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/43f0022fcaed2d9d1b1b3707173acd562c7f0ebe

tdf#155447 macOS a11y: Report accessibilityIdentifier

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 14 Commit Notification 2024-07-24 16:21:27 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/84929c43df529ad9fcb6382349015b3d9aa2e202

tdf#155447 qt a11y: Map TEXT_FRAME role to QAccessible::Pane

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 15 Commit Notification 2024-08-02 04:48:08 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/0dd7751df292f5135be13b13e74e415b20053c67

tdf#155447 a11y: Introduce weld::Widget::get_accessible_id()

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 16 Commit Notification 2024-08-02 04:48:11 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/57f6105c90019f6a2070ab52335d7a4e852466ff

tdf#155447 a11y: Set accessible ID for WeldEditView

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 17 Commit Notification 2024-08-02 04:48:13 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/1d1640dbeda76fcecd7b81200a60c267cea5f2f7

tdf155447 gtk3 a11y: Report accessible ID for drawing area

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 18 Michael Weghorn 2024-08-06 10:51:16 UTC
(In reply to Michael Weghorn from comment #11)
> "SpellingDialog" is the accessible ID for the dialog.
> Orca MR demonstrating how this can be used:
> 
> [...]
> 
> @joanie: What do you think?

For now, I'll take merging the first MR as approval. :-)
Please please speak up if you disagree.

Second MR making use of the accessible ID when identifying the suggestions list and widget displaying the error:
https://gitlab.gnome.org/GNOME/orca/-/merge_requests/224
Comment 19 Joanmarie Diggs 2024-08-06 19:46:56 UTC
Hey Michael.
Sorry I missed your request for comment here. I'm not working this week, but I saw your merge request. I'm afraid I think "suggestionslb" is not an optimal identifier.

Please note that the ultimate goal is for the identifiers to be embraced/standardized by all apps with spell-check dialogs; not just LO.

1. What if some app (or even LO in the future) stops using a listbox for the suggestions list?

2. Even if everyone for now and forever would only use a listbox, does the role need to be part of the identifier? It's already part of the role. (Nit-picky aside: If the role did need to be part of the identifier, "lb" isn't very clear.)

3. If everyone embraces your approach to use identifiers, shouldn't the collection of spelling suggestions have "spelling" in its identifier?

I'm super sorry I didn't notice this until after you landed it and then did an Orca merge request. :( Is it too late to tweak the identifier(s)?
Comment 20 Michael Weghorn 2024-08-07 08:50:54 UTC
Created attachment 195751 [details]
Screenshot of "id" object attribute used in Thunderbird's spell check dialog
Comment 21 Michael Weghorn 2024-08-07 09:05:55 UTC
(In reply to Joanmarie Diggs from comment #19)
> Sorry I missed your request for comment here. I'm not working this week, but
> I saw your merge request. I'm afraid I think "suggestionslb" is not an
> optimal identifier.
> [...]
> 
> Is it too late to tweak the identifier(s)?

I totally agree it's not an ideal identifier, and changing it sounds like a good idea indeed. It's trivial to do by now.

The previous commits for this ticket implemented the logic that the id as set in the .ui file is reported as accessible ID as well.
"suggestionslb" is just what happened to be used there already, so was not even a deliberate choice.

We can still change the IDs for the 3 UI elements in question to whatever makes the most sense.

In other news, I've just taken a look at the Thunderbird spell check dialog, and while it does not report an "AccessibleId" property (as can be retrieved by `Atspi.Accessible.get_accessible_id`), the dialog itself and the UI elements have an "id" object attribute set, see screenshot attachment 195751 [details] for the example of the suggestions list. It currently uses these IDs:

* dialog: "spellCheckDlg"
* misspelled word: "MisspelledWord"
* suggestions list: "SuggestedList"

So in principle, it already supports the same concept/approach and Orca could presumably make use of those IDs, similar to what my current MR implements for LO.

I can think of the following potential approaches now:

1) Think about "proper" IDs, use them in LO and suggest to use them in Thunderbird as well.

2) Align the LO ones with the Thunderbird ones. (But I'm not sure whether "spellCheckDlg" really sounds like something to standardize on... :)

The following could then make sense in Orca:

A) Support both, the "AccessibleId" property and the "id" object attribute, so both approaches work. (If necessary, we could alternatively also report an "id" object attribute in LO in addition.)

B) In case of 1) above, additionally supporting the IDs that Thunderbird currently uses at least until transition to new "standardized" names is finished could be an option, i.e. let Orca check the ID against a list of supported IDs.

What's different in LO as compared to Thunderbird is that Thunderbird just has a label with the misspelled text, while LO has a panel that has a paragraph child containing the relevant text (and multiple paragraphs are supported, e.g. if the user decides to type more text) and the ID is set for the panel, not for the paragraph, so that might still need some distinguished handling in Orca.

What do you think? Do you have any suggestions for the names to use?
Comment 22 Michael Weghorn 2024-08-07 09:18:51 UTC
(In reply to Joanmarie Diggs from comment #19)
> Sorry I missed your request for comment here. I'm not working this week [...]
And please don't worry about that. The remaining aspects are also not urgent at all! :)
Comment 23 Michael Weghorn 2024-08-07 09:24:05 UTC
(In reply to Michael Weghorn from comment #21)
> Do you have any suggestions for the names to use?

Once decided upon, I'm wondering whether there would be a good place to document those IDs, e.g. somewhere in AT-SPI doc or maybe Orca's `README-APPLICATION-DEVELOPERS.md`? Then it would be easy to refer to that when submitting corresponding changes or issues for Thunderbird and other applications.
Comment 24 Commit Notification 2024-08-29 07:33:00 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/21b29f25660db973fa1480de77e6a69d76a5de53

tdf#155447 wina11y: Report accessible ID as "id" obj attribute

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 25 Michael Weghorn 2024-08-29 07:52:53 UTC
(In reply to Michael Weghorn from comment #2)
> For IAccessible2, that could still be mapped to an "accessible-id:<id>;"
> atttribute or somesuch if considered useful.

For IAccessible2, https://git.libreoffice.org/core/commit/21b29f25660db973fa1480de77e6a69d76a5de53 introduces the use of an object attribute just called "id", and that attribute is now also documented in the upstream IAccessible2 text attribute spec:
https://github.com/LinuxA11y/IAccessible2/commit/2b8c2c79417bad4b464761a142fab45ffde8bfa8
Comment 26 Michael Weghorn 2024-08-29 08:00:55 UTC
(In reply to Michael Weghorn from comment #25)
> (...) that attribute is now also documented in the
> upstream IAccessible2 text attribute spec: (...)

It's described in the **object** attribute spec, of course, not the text attribute spec.
Comment 27 Michael Weghorn 2024-11-07 14:52:31 UTC
Closing as fixed.

Orca implementation that works with LO and could be reused for more apps is in this commit:

https://gitlab.gnome.org/GNOME/orca/-/commit/40a2d302

commit 40a2d302eb52295433fd84e6c254a7dbe5108a24
Author: Joanmarie Diggs
Date:   Thu Nov 7 14:15:07 2024 +0100

    Spellcheck: Check for accessible id in more places
    
    Do the following case-insensitive checks:
    * If the object's accessible id starts with "suggestions" treat it
      as the suggestions list.
    * If the object's accessible id starts with "replacement" treat it
      as the object (likely entry) which contains the proposed replacement.
    * If the label's/widget's accessible id starts with "error" treat
      it as the container displaying the misspelled word.
    
    Note that the first of the three is based on what LO 25.2 currently
    exposes ("suggestionslb"). The other two are not in use yet, but adding
    them facilitates implementation in, and getting feedback from, apps and
    toolkits.
    
    Also modify the existing check for the window. We were doing an exact
    match on "SpellingDialog". Making that case insensitive and limiting to
    starts with "spelling" works with the current LO implementation and
    removes an implementation detail ("dialog").

If necessary, IDs on LO side could be tweaked further on demand.
Comment 28 Joanmarie Diggs 2024-11-07 15:04:09 UTC
The id Orca is expecting for the error doesn't match what you have in place, right?
Comment 29 Michael Weghorn 2024-11-08 13:03:14 UTC
(In reply to Joanmarie Diggs from comment #28)
> The id Orca is expecting for the error doesn't match what you have in place,
> right?

Indeed, sorry for missing that.

Pending changes to adjust that, and also add a test for the expected accessible IDs, so this doesn't break without noticing in the future:

https://gerrit.libreoffice.org/c/core/+/176280
https://gerrit.libreoffice.org/c/core/+/176281
Comment 30 Commit Notification 2024-11-08 15:55:19 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/4b7316e2ff6e63bbf73e0bcf04f90b807d9829ad

tdf#155447 a11y: Set accessible ID that Orca expects

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 31 Commit Notification 2024-11-08 15:55:22 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/39b15dc8f0adec9e856a39cf319c7c8ae6750043

tdf#155447 a11y: Add test for accessible IDs expected by Orca

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 32 Michael Weghorn 2024-11-08 15:57:03 UTC
(In reply to Michael Weghorn from comment #29)
> Pending changes to adjust that, and also add a test for the expected
> accessible IDs, so this doesn't break without noticing in the future:
> 
> https://gerrit.libreoffice.org/c/core/+/176280
> https://gerrit.libreoffice.org/c/core/+/176281

Merged now. Please let me know if anything else is missing when using a current LO development version.