Bug 75162 - [Accessibility] the root of a LibreOffice document must be of role "document", not "window"
Summary: [Accessibility] the root of a LibreOffice document must be of role "document"...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.3.0.0.alpha0+ Master
Hardware: Other Windows (All)
: medium major
Assignee: Not Assigned
URL:
Whiteboard: target:4.3.0
Keywords:
Depends on:
Blocks: a11y, Accessibility
  Show dependency treegraph
 
Reported: 2014-02-18 15:52 UTC by Marlin
Modified: 2016-10-25 20:08 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments
AccProbe session showing the window vs. document AccessibleRole (113.11 KB, image/jpeg)
2014-02-19 17:54 UTC, V Stuart Foote
Details
AccProbe session from 4.2.1.1 showing doccument accessible role (118.42 KB, image/jpeg)
2014-02-19 20:58 UTC, V Stuart Foote
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Marlin 2014-02-18 15:52:12 UTC
Using NVDA to edit and read a text in Libreoffice Writer, when inserting a new line with enter key or arrowing down or up through the lines, the screen reader always announces "Paragraph Editable Multi-line", which is not really useful, and definitely bothers me and I think this bothers any screen reader user as well.
Comment 1 Marlin 2014-02-18 15:56:35 UTC
Version 4.2 with IAccessible enabled doesn't presents this bug.
Comment 2 V Stuart Foote 2014-02-19 03:37:25 UTC
@Marlin, thanks for reporting. Helpful if you can include the LODev build detail, OS and the buidld of the Assistive Technology tool you are using.

Confirming on Windows 7 sp1, 64-bit with NVDA 2013.3

Version: 4.3.0.0.alpha0+
Build ID: 637353bb46d3c7d9537e47e4e003aef78a0c0ab3
TinderBox: Win-x86@39, Branch:master, Time: 2014-02-19_01:39:31

a nuisance but correct, can't this be adjusted in the AT? (just moved onto NVDA 2013.3 so not sure yet about the application profiles.)
Comment 3 V Stuart Foote 2014-02-19 03:38:17 UTC
setting as NEW, but probably is NOT A BUG
Comment 4 Marco Zehe 2014-02-19 05:38:01 UTC
This announcement is essentially correct. Each paragraph in an Office Writer document is a new accessible element, so when moving from one paragraph to the next, a focus change occurs.

Note that 4.2 doesn't present this bug probably due to bug 74232 and bug 74234, e. g. the IA2 support there is somehow still incomplete.
Comment 5 Marlin 2014-02-19 09:52:21 UTC
@Marco, Although this is an accessibility information, It's so tedious to be listening this message always you move by line.
If you make a survey, all blinds will confirm this.
By the way, this is not present in other text editors, including Open-office.
I'm using NVDA version 2014 dev, also called "next".

My Libreoffice version is the last 47 tdf build, although I have already tested using the build 39.
Comment 6 Marco Zehe 2014-02-19 10:09:21 UTC
I just took a look at the soffice.py appModule for NVDA, and believe this may have something to do with a wider-ranging bug in LibreOffice where the IAccessibleText interface is not being exposed. The states I see in OpenOffice and LibreOffice for a paragraph are identical, but in OpenOffice, NVDA suppresses some information, in LibreOffice, it doesn't. There are quite a number of instances where NVDA tests for the presence of the IAccessibleText interface and supposedly fails.

A bug most likely related to this is bug 74242, which also shows a variation of the IAccessibleText interface not being present or properly exposed by LibreOffice.

So yeah, Marlin, you're right, we do have a bug here, and quite a serious one, too.
Comment 7 Marco Zehe 2014-02-19 16:38:42 UTC
OK, I found the cause. The document root is a window accessible when it should be a document accessible.

STR:
1. Open LibreOffice 4.3.0-dev and open any document in Writer.
2. Launch accProbe.
3. In the Explorer pane, find the RootPane accessible that has the title of your document as its name, and expand it.
4. Find the first collapsed OptionPane, in my case OptionPane[4] and open it.
5. Open the Scrollpane which is the first child right below.
6. Next, yu'll see a node labeled Window, followed by the name of your document. This is the root of the document, all other things that are contained within the document, go below that.

The role of this accessible should not be "window", but "document" instead. The wrong role confuses NVDA and other ATS, they effectively don't know that they're in a document environment. A Window role is not appropriate here because a window is a much larger unit, with toolbars, menu bars, status bars and other controls. This particular unit must be an accessible of role "document".
Comment 8 Marco Zehe 2014-02-19 16:41:10 UTC
Caolán, is what I described in comment #7 easy to fix?
Comment 9 V Stuart Foote 2014-02-19 17:54:23 UTC
Created attachment 94373 [details]
AccProbe session showing the window vs. document AccessibleRole

(In reply to comment #7)
I've attached a screen clip of the issue in AccProbe.
Comment 10 V Stuart Foote 2014-02-19 19:53:00 UTC
The window vs document accessible roles look to be at the MSAA mapping of UNO API roles point. Those mapping occur in /core/winaccessibility/source/service/AccObject.cxx

Is the change a simple enum tweak, or further down into the UNO API guts?

Michael M and Michael S worked on this part of the IA2 merge. 

Does the note from Steve Yin at line 226:  "In order to windows API WindowFromAccessibleObject,we sometimes to set a pure top window accessible object created by windows system as top ancestor"  refer to the rootPane of a running LibreOffice session, or to this MSAA 'window' role that may need to be adjusted to 'document'?
Comment 11 Michael Meeks 2014-02-19 20:28:05 UTC
Interesting; we had a patch recently from Jacobo:

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2bb87dd6596f8e961f5618d8e7686fa12c64e2cf

Adding a number of more descriptive document roles for use in ATK; then again - I'm not sure which peer in the hierarchy those are associated with.
Comment 12 Michael Meeks 2014-02-19 20:35:20 UTC
Poking at:

void AccObject::UpdateRole()
{
    XAccessibleContext* pContext  = m_xAccContextRef.get();
    m_pIMAcc->Put_XAccRole( ROLE_SYSTEM_WINDOW  );
    short iRoleIndex = pContext->getAccessibleRole();
    if ((0 <= iRoleIndex) && (iRoleIndex < SAL_N_ELEMENTS(ROLE_TABLE)))
    {
        short iIA2Role = ROLE_TABLE[iRoleIndex][1] ;
        m_pIMAcc->Put_XAccRole( iIA2Role  );
    }

}

it strikes me that we're missing a number of entries in the ROLE_TABLE, particularly for the new roles; such that:

diff --git a/winaccessibility/source/service/AccObject.cxx b/winaccessibility/source/service/AccObject.cxx
index 3897269..c6573f1 100644
--- a/winaccessibility/source/service/AccObject.cxx
+++ b/winaccessibility/source/service/AccObject.cxx
@@ -124,13 +124,18 @@ const short ROLE_TABLE[][2] =
         {CHART,                     IA2_ROLE_SHAPE},
         {EDIT_BAR,                  IA2_ROLE_EDITBAR},
         {FORM,                      IA2_ROLE_FORM},
-        {IMAGE_MAP ,              IA2_ROLE_IMAGE_MAP},
+        {IMAGE_MAP ,                IA2_ROLE_IMAGE_MAP},
         {NOTE,                      IA2_ROLE_NOTE},
         {PAGE,                      IA2_ROLE_PAGE},
-        {RULER ,                  IA2_ROLE_RULER},
+        {RULER ,                    IA2_ROLE_RULER},
         {SECTION,                   IA2_ROLE_SECTION},
-        {TREE_ITEM ,              ROLE_SYSTEM_OUTLINEITEM},
+        {TREE_ITEM ,                ROLE_SYSTEM_OUTLINEITEM},
         {TREE_TABLE,                ROLE_SYSTEM_OUTLINE}
+        {COMMENT,                   ROLE_SYSTEM_WINDOW /* ?FIXME? */ },
+        {COMMENT_END,               ROLE_SYSTEM_WINDOW /* ?FIXME? */ },
+        {DOCUMENT_PRESENTATION,     ROLE_SYSTEM_DOCUMENT },
+        {DOCUMENT_SPREADSHEET,      ROLE_SYSTEM_DOCUMENT },
+        {DOCUMENT_TEXT,             ROLE_SYSTEM_DOCUMENT },
     };
 
 

might help here - thoughts / testing appreciated. I'd also love input on the COMMENT / COMMENT_END mappings - cf.

http://cgit.freedesktop.org/libreoffice/core/tree/offapi/com/sun/star/accessibility/AccessibleRole.idl#n682

Which also appear to be missing.
Comment 13 V Stuart Foote 2014-02-19 20:58:31 UTC
Created attachment 94382 [details]
AccProbe session from 4.2.1.1 showing doccument accessible role

(In reply to comment #11)
> Interesting; we had a patch recently from Jacobo:
...
> Adding a number of more descriptive document roles for use in ATK; then
> again - I'm not sure which peer in the hierarchy those are associated with.

Hmm, I just ran the same review with AccProbe with the
Version: 4.2.1.0.0+ build
Build ID: 9a19e8d838753128504274e1885eb3ce8ec1dbb8
TinderBox: Win-x86@42, Branch:libreoffice-4-2, Time: 2014-01-30_00:20:59

It looks to be getting the preferred Document role at that point, but is not getting the Document title as shown in the rootPane as its accessibleName, and gets only an accessible name of 'Document view'

A check of a build of AOO 4.1.0 dev build (r1562493) has the accessibleName assigned the document name.
Comment 14 Marco Zehe 2014-02-20 07:28:17 UTC
(In reply to comment #12)
> +        {COMMENT,                   ROLE_SYSTEM_WINDOW /* ?FIXME? */ },
> +        {COMMENT_END,               ROLE_SYSTEM_WINDOW /* ?FIXME? */ },

Jamie, what are the best mappings for these two?

Perhaps our role map from Firefox can also help here:
http://lxr.mozilla.org/mozilla-central/source/accessible/src/base/RoleMap.h

> void AccObject::UpdateRole()
> {
>     XAccessibleContext* pContext  = m_xAccContextRef.get();
>     m_pIMAcc->Put_XAccRole( ROLE_SYSTEM_WINDOW  );
>     short iRoleIndex = pContext->getAccessibleRole();
>     if ((0 <= iRoleIndex) && (iRoleIndex < SAL_N_ELEMENTS(ROLE_TABLE)))
>     {
>         short iIA2Role = ROLE_TABLE[iRoleIndex][1] ;
>         m_pIMAcc->Put_XAccRole( iIA2Role  );
>     }
> 
> }

Michael, I agree that this method has an odd accessible role to fall through. Should probably check for the whole map and use the highest available role, not one in the middle.
Comment 15 James Teh 2014-02-20 10:43:17 UTC
(In reply to comment #14)
> > +        {COMMENT,                   ROLE_SYSTEM_WINDOW /* ?FIXME? */ },
> > +        {COMMENT_END,               ROLE_SYSTEM_WINDOW /* ?FIXME? */ },
> Jamie, what are the best mappings for these two?
First, ROLE_SYSTEM_WINDOW probably shouldn't be used by anything else except oleacc. This role generally refers to the window itself, including the non-client areas such as the title bar, system menu bar, application menu bar, etc. If you need a fallback role for text stuff, I'd probably go with IA2_ROLE_TEXT_FRAME.

As for these two, the best I can come up with for now is IA2_ROLE_NOTE. We've never had to map these before. If it becomes an issue, we may need to try to get new roles into the IA2 spec.
Comment 16 Marco Zehe 2014-02-20 14:28:23 UTC
Thanks Jamie! Note that in the current checkout I have of the source, the above mentioned new items Michael mentions are not yet part of the build. Michael, is this something you have locally? If so, I'd suggest you go with what Jamie said.
Comment 17 Commit Notification 2014-02-22 22:12:46 UTC
Michael Meeks committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=b129443eeddab30b31c07bb8c544adaa80785046

fdo#75162 - IA2: map new document roles to ROLE_SYSTEM_DOCUMENT.



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 18 Michael Meeks 2014-02-22 22:13:44 UTC
Marco: sure, I've not compile tested that - so ... anyhow, I've pushed it just now, on your advice =) using the TEXT_FRAME input from James for comments - worth testing how they show up I guess. Glad you have a checkout locally - do you have a build ?

Anyhow - would appreciate testing & closing this if it fixes things for people.
Comment 19 Marlin 2014-02-23 13:40:43 UTC
Tested and fixed. Thanks!
Comment 20 V Stuart Foote 2014-02-23 14:48:52 UTC
On Windows 7 sp1, 64-bit

Confirming the accessible role has changed to 'docuemnt' and paragraphs back to announcing as 'blank' with /A Administrative installs both of todays TB39 and TB47 builds of master on Windows

Version: 4.3.0.0.alpha0+
Build ID: b129443eeddab30b31c07bb8c544adaa80785046
TinderBox: Win-x86@39, Branch:master, Time: 2014-02-23_00:07:48

Version: 4.3.0.0.alpha0+
Build ID: f839b5dd16c05c0eda21345ec36ec0cb024eb732
TinderBox: Win-x86@47-TDF, Branch:MASTER, Time: 2014-02-23_09:54:38

The TB47, with the en-US help files installed, has dictionary support and command annotations available, so maybe is a better choice for testing accessibility UX.  Of course TB39 with debug symbols available will still be needed for that use.