Bug 72639 - ACCESSIBILITY: Recent documents in the new start center don't send the proper a11y events
Summary: ACCESSIBILITY: Recent documents in the new start center don't send the proper...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
4.2.0.0.beta2
Hardware: Other All
: highest normal
Assignee: Jacobo Aragunde Pérez
QA Contact:
URL:
Whiteboard: Confirmed:4.2.0.0.beta2:Ubuntu target...
Keywords: accessibility
Depends on:
Blocks: a11y mab4.2
  Show dependency treegraph
 
Reported: 2013-12-12 12:10 UTC by Jacobo Aragunde Pérez
Modified: 2016-10-25 19:57 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
accessible-event listener (516 bytes, text/x-python)
2013-12-12 12:10 UTC, Jacobo Aragunde Pérez
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jacobo Aragunde Pérez 2013-12-12 12:10:27 UTC
Created attachment 90662 [details]
accessible-event listener

Steps to reproduce:
1. Launch the attached accessible-event listener in a terminal.
2. Launch LibreOffice Start Center and press tab until you move into the recent documents area.
3. Now use the arrow to move between documents.

You will see in the listener log that the selection-changed event is launched and it has the manages-descendants property set. That property means, according to ATK documentation [1], that the object should handle the state notifications of its descendants and it should trigger active-descendant-changed events. As you can check in the listener log, these events aren't being emitted.

Options to fix this bug:
1. Send the missing active-descendant-changed events.
2. Disable manages-descendants property if it's not really necessary.


[1] https://developer.gnome.org/atk/stable/atk-AtkState.html
Comment 1 Robinson Tryon (qubit) 2013-12-18 14:39:48 UTC
CONFIRMED on 4.2.0.0.beta2 + Ubuntu 12.04.3

(In reply to comment #0)
> Steps to reproduce:
> 1. Launch the attached accessible-event listener in a terminal.
> 2. Launch LibreOffice Start Center and press tab until you move into the
> recent documents area.
> 3. Now use the arrow to move between documents.
> 
> You will see in the listener log that the selection-changed event is
> launched and it has the manages-descendants property set. That property
> means, according to ATK documentation [1], that the object should handle the
> state notifications of its descendants and it should trigger
> active-descendant-changed events. As you can check in the listener log,
> these events aren't being emitted.

Yep -- I don't see any active-descendant-changed events in the log.


Status: NEW
Whiteboard: a11y
Comment 2 Robinson Tryon (qubit) 2013-12-19 16:19:20 UTC
Add to MAB4.2
Comment 3 V Stuart Foote 2014-01-04 15:08:14 UTC
Believe issue originates in handling keyboard movements in the thumbnailview.cxx code. Affects both StartCenter and Template Manager, bug 73243
Comment 4 Björn Michaelsen 2014-01-17 09:40:32 UTC Comment hidden (obsolete)
Comment 5 Jacobo Aragunde Pérez 2014-01-20 13:37:25 UTC
I've been taking a look at the code.

ThumbnailView::SelectItem is where code to launch the event ACTIVE_DESCENDANT_CHANGED is located. In particular, that happens here:


    // focus event (select)
    ThumbnailViewAcc* pItemAcc = ThumbnailViewAcc::getImplementation( pItem->GetAccessible( mbIsTransientChildrenDisabled ) );

    if( pItemAcc )
    {
        ::com::sun::star::uno::Any aOldAny, aNewAny;
        if( !mbIsTransientChildrenDisabled )
        {
            aNewAny <<= ::com::sun::star::uno::Reference< ::com::sun::star::uno::XInterface >(
                static_cast< ::cppu::OWeakObject* >( pItemAcc ));
            ImplFireAccessibleEvent( ::com::sun::star::accessibility::AccessibleEventId::ACTIVE_DESCENDANT_CHANGED, aOldAny, aNewAny );
        }
        else
        {
            aNewAny <<= ::com::sun::star::accessibility::AccessibleStateType::FOCUSED;
            pItemAcc->FireAccessibleEvent( ::com::sun::star::accessibility::AccessibleEventId::STATE_CHANGED, aOldAny, aNewAny );
        }
    }

The value of mbIsTransientChildrenDisabled is correct, so the code should enter here and launch the event we want... except for the value of pItemAcc, which is zero.

And pItemAcc is zero because ThumbnailViewAcc::getImplementation calls ThumbnailViewItemAcc::getSomething, which returns zero because the result of memcmp is not zero, which means the memory areas are not equal.
Comment 6 Jacobo Aragunde Pérez 2014-01-20 16:37:40 UTC
(In reply to comment #5)
> ... ThumbnailViewAcc::getImplementation calls
> ThumbnailViewItemAcc::getSomething, which returns zero because the result of
> memcmp is not zero, which means the memory areas are not equal.

In the end, the two memory areas compared are the result of ThumbnailViewItemAcc.getUnoTunnelId() and ThumbnailViewAcc.getUnoTunnelId(). This is weird, because both come from the same rtl::Static class, called theValueSetAccUnoTunnelId, isn't it?
Comment 7 Jacobo Aragunde Pérez 2014-01-20 19:34:21 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > ... ThumbnailViewAcc::getImplementation calls
> > ThumbnailViewItemAcc::getSomething, which returns zero because the result of
> > memcmp is not zero, which means the memory areas are not equal.
> 
> In the end, the two memory areas compared are the result of
> ThumbnailViewItemAcc.getUnoTunnelId() and ThumbnailViewAcc.getUnoTunnelId().
> This is weird, because both come from the same rtl::Static class, called
> theValueSetAccUnoTunnelId, isn't it?

The implementation of getSomething was correct according to the definition of XUnoTunnel interface.

The problem was in the original implementation of ThumbnailView::SelectItem, where the object ThumbnailViewAcc was being mistakenly uses instead of ThumbnailViewItemAcc.

I've sent a patch for review: https://gerrit.libreoffice.org/7551
Comment 8 V Stuart Foote 2014-01-20 20:06:24 UTC
@Jacobo,
(In reply to comment #7)
> 
> The implementation of getSomething was correct according to the definition
> of XUnoTunnel interface.
> 
> The problem was in the original implementation of ThumbnailView::SelectItem,
> where the object ThumbnailViewAcc was being mistakenly uses instead of
> ThumbnailViewItemAcc.
> 
> I've sent a patch for review: https://gerrit.libreoffice.org/7551

Great! This might also resolve bug 73243 -- Kendy, Zolnai would you be able to review and commit? And if functional, resolving the thumbnail view accessibility events, this would be a good candidate to push down to 4.2.0.
Comment 9 Commit Notification 2014-01-21 15:15:22 UTC
Jacobo Aragunde Perez committed a patch related to this issue.
It has been pushed to "master":

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

fdo#72639: send proper ACTIVE_DESCENDANT_CHANGED events



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 10 Commit Notification 2014-01-21 15:17:47 UTC
Jacobo Aragunde Perez committed a patch related to this issue.
It has been pushed to "libreoffice-4-2":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=cdc6d32d9062b1c143b094910bc913cb8f183afa&h=libreoffice-4-2

fdo#72639: send proper ACTIVE_DESCENDANT_CHANGED events


It will be available in LibreOffice 4.2.1.

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 11 Michael Stahl 2014-01-21 15:30:13 UTC
just for the record, i hate you bugzilla
Comment 12 Commit Notification 2014-01-21 15:39:28 UTC
Jacobo Aragunde Perez committed a patch related to this issue.
It has been pushed to "libreoffice-4-2-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=eed45184ff542a713d28928b50c5a8ce8efd3a92&h=libreoffice-4-2-0

fdo#72639: send proper ACTIVE_DESCENDANT_CHANGED events


It will be available already in LibreOffice 4.2.0.

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 13 V Stuart Foote 2014-01-22 19:14:23 UTC
On Windows 7 sp1 64-bit with
Version: 4.3.0.0.alpha0+
Build ID: 77637324abc193d831bb4a0fa6f9a91ef3601960
TinderBox: Win-x86@39, Branch:master, Time: 2014-01-22_16:19:04

Confirming fixed on master.
Comment 14 Robinson Tryon (qubit) 2015-12-18 09:34:16 UTC Comment hidden (obsolete)