Changeset 170008 in webkit


Ignore:
Timestamp:
Jun 16, 2014, 8:01:06 AM (11 years ago)
Author:
mario.prada@samsung.com
Message:

[ATK] Missing 'selection-changed' signal when navigating a combo box with keyboard
​https://bugs.webkit.org/show_bug.cgi?id=133512

Reviewed by Chris Fleizach.

Source/WebCore:
Make sure that AccessibilityMenuList objects update their active
option when it changes, which will send a platform-dependent
accessibility-related notification when needed.

Test: accessibility/combo-box-collapsed-selection-changed.html

  • rendering/RenderMenuList.cpp:

(RenderMenuList::didUpdateActiveOption): Keep the out-of-bounds
check for the index passed but don't avoid updating the option for
the associated AccessibilityMenuList object if the selected list
item does not have a renderer, because that could be the case for
cases where the popup (and its elements) would be rendered in the
UI Process (e.g. GTK+ port uses GtkMenu and GtkMenuItem for that).

  • accessibility/AccessibilityMenuList.cpp:

(WebCore::AccessibilityMenuList::didUpdateActiveOption): Ensure
that the AccessibilityMenuListPopup object for a given menu list
has accessibility children before updating its active option.

Tools:
Added support for connecting to AtkSelection's 'selection-changed'
signal, and print it out as AXSelectedChildrenChanged in the tests.

Also removed some dead code, that became useless after r169487.

  • WebKitTestRunner/InjectedBundle/atk/AccessibilityNotificationHandlerAtk.cpp:

(WTR::AccessibilityNotificationHandler::connectAccessibilityCallbacks): Updated.

LayoutTests:
Re-implemented test for combo boxes in terms of addNotificationListener()
instead of using the (already deprecated) logAccessibilityEvents method,
and made the test cross platform (as the fix is not platform specific).

  • accessibility/combo-box-collapsed-selection-changed.html:

Implemented based on the former gtk-only test, and made it cross-platform.

  • accessibility/combo-box-collapsed-selection-changed-expected.txt: New.
  • platform/gtk/accessibility/combo-box-collapsed-selection-changed.html: Removed.
  • platform/gtk/accessibility/combo-box-collapsed-selection-changed-expected.txt: Removed.

Updated expectation for test that checks that a notification is
sent when navigating through a multiselection list box, now that
we are actually printing such a notification.

  • accessibility/multiselect-list-reports-active-option-expected.txt: Updated.

Removed two expected failures from TestExpectations for tests that
are now passing, one for the combo box test mentioned above and
another one for a test that is passing as well now, after applying
this fix: accessibility/menu-list-sends-change-notification.html

  • platform/gtk/TestExpectations: Removed two 'failure' expectations.
  • platform/mac/TestExpectations: Skip accessiblity test timing out, probably because

those kind of notifications while navigating a combo box are not needed in the Mac.

Location:
trunk
Files:
2 added
2 deleted
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r170005 r170008  
     12014-06-16  Mario Sanchez Prada  <mario.prada@samsung.com>
     2
     3        [ATK] Missing 'selection-changed' signal when navigating a combo box with keyboard
     4        https://bugs.webkit.org/show_bug.cgi?id=133512
     5
     6        Reviewed by Chris Fleizach.
     7
     8        Re-implemented test for combo boxes in terms of addNotificationListener()
     9        instead of using the (already deprecated) logAccessibilityEvents method,
     10        and made the test cross platform (as the fix is not platform specific).
     11
     12        * accessibility/combo-box-collapsed-selection-changed.html:
     13        Implemented based on the former gtk-only test, and made it cross-platform.
     14        * accessibility/combo-box-collapsed-selection-changed-expected.txt: New.
     15        * platform/gtk/accessibility/combo-box-collapsed-selection-changed.html: Removed.
     16        * platform/gtk/accessibility/combo-box-collapsed-selection-changed-expected.txt: Removed.
     17
     18        Updated expectation for test that checks that a notification is
     19        sent when navigating through a multiselection list box, now that
     20        we are actually printing such a notification.
     21
     22        * accessibility/multiselect-list-reports-active-option-expected.txt: Updated.
     23
     24        Removed two expected failures from TestExpectations for tests that
     25        are now passing, one for the combo box test mentioned above and
     26        another one for a test that is passing as well now, after applying
     27        this fix: accessibility/menu-list-sends-change-notification.html
     28
     29        * platform/gtk/TestExpectations: Removed two 'failure' expectations.
     30
     31        * platform/mac/TestExpectations: Skip accessiblity test timing out, probably because
     32        those kind of notifications while navigating a combo box are not needed in the Mac.
     33
    1342014-06-16  Frédéric Wang  <fred.wang@free.fr>
    235

  • trunk/LayoutTests/accessibility/multiselect-list-reports-active-option-expected.txt

    r160095 r170008  
    2323PASS accessibleThree.isSelected is true
    2424PASS accessibleThree.isSelectedOptionActive is true
     25List notification: AXSelectedChildrenChanged
     26List notification: AXSelectedChildrenChanged
    2527
    2628TEST COMPLETE

  • trunk/LayoutTests/platform/gtk/TestExpectations

    r169940 r170008  
    16821682webkit.org/b/113772 http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_cached.html [ Failure ]
    16831683
    1684 webkit.org/b/114259 platform/gtk/accessibility/combo-box-collapsed-selection-changed.html [ Failure ]
    1685 
    16861684webkit.org/b/115025 fast/events/constructors/mouse-event-constructor.html [ Failure ]
    16871685webkit.org/b/115025 fast/events/constructors/wheel-event-constructor.html [ Failure ]
    … …  
    17861784
    17871785webkit.org/b/125406 fast/regions/relative-in-absolute-borders-overflow.html [ ImageOnlyFailure ]
    1788 
    1789 webkit.org/b/126521 accessibility/menu-list-sends-change-notification.html [ Failure ]
    17901786
    17911787webkit.org/b/126619 http/tests/media/video-auth.html [ Failure ]

  • trunk/LayoutTests/platform/mac/TestExpectations

    r170005 r170008  
    4242# Accessibility tests for notifications that don't exist or aren't needed on Mac OS X.
    4343accessibility/aria-checkbox-sends-notification.html
     44accessibility/combo-box-collapsed-selection-changed.html
    4445accessibility/children-changed-sends-notification.html
    4546accessibility/menu-list-sends-change-notification.html

  • trunk/Source/WebCore/ChangeLog

    r170007 r170008  
     12014-06-16  Mario Sanchez Prada  <mario.prada@samsung.com>
     2
     3        [ATK] Missing 'selection-changed' signal when navigating a combo box with keyboard
     4        https://bugs.webkit.org/show_bug.cgi?id=133512
     5
     6        Reviewed by Chris Fleizach.
     7
     8        Make sure that AccessibilityMenuList objects update their active
     9        option when it changes, which will send a platform-dependent
     10        accessibility-related notification when needed.
     11
     12        Test: accessibility/combo-box-collapsed-selection-changed.html
     13
     14        * rendering/RenderMenuList.cpp:
     15        (RenderMenuList::didUpdateActiveOption): Keep the out-of-bounds
     16        check for the index passed but don't avoid updating the option for
     17        the associated AccessibilityMenuList object if the selected list
     18        item does not have a renderer, because that could be the case for
     19        cases where the popup (and its elements) would be rendered in the
     20        UI Process (e.g. GTK+ port uses GtkMenu and GtkMenuItem for that).
     21
     22        * accessibility/AccessibilityMenuList.cpp:
     23        (WebCore::AccessibilityMenuList::didUpdateActiveOption): Ensure
     24        that the AccessibilityMenuListPopup object for a given menu list
     25        has accessibility children before updating its active option.
     26
    1272014-06-16  Commit Queue  <commit-queue@webkit.org>
    228

  • trunk/Source/WebCore/accessibility/AccessibilityMenuList.cpp

    r167024 r170008  
    114114        ASSERT(childObjects[0]->isMenuListPopup());
    115115
    116         if (childObjects[0]->isMenuListPopup()) {
     116        // We might be calling this method in situations where the renderers for list items
     117        // associated to the menu list have not been created (e.g. they might be rendered
     118        // in the UI process, as it's the case in the GTK+ port, which uses GtkMenuItem).
     119        // So, we need to make sure that the accessibility popup object has some children
     120        // before asking it to update its active option, or it will read invalid memory.
     121        // You can reproduce the issue in the GTK+ port by removing this check and running
     122        // accessibility/insert-selected-option-into-select-causes-crash.html (will crash).
     123        if (childObjects[0]->isMenuListPopup() && childObjects[0]->children().size()) {
    117124            if (AccessibilityMenuListPopup* popup = toAccessibilityMenuListPopup(childObjects[0].get()))
    118125                popup->didUpdateActiveOption(optionIndex);

  • trunk/Source/WebCore/rendering/RenderMenuList.cpp

    r169591 r170008  
    437437        return;
    438438
    439     HTMLElement* listItem = selectElement().listItems()[listIndex];
    440     ASSERT(listItem);
    441     if (listItem->renderer()) {
    442         if (AccessibilityMenuList* menuList = toAccessibilityMenuList(document().axObjectCache()->get(this)))
     439    if (AXObjectCache* cache = document().existingAXObjectCache()) {
     440        if (AccessibilityMenuList* menuList = toAccessibilityMenuList(cache->get(this)))
    443441            menuList->didUpdateActiveOption(optionIndex);
    444442    }

  • trunk/Tools/ChangeLog

    r169997 r170008  
     12014-06-16  Mario Sanchez Prada  <mario.prada@samsung.com>
     2
     3        [ATK] Missing 'selection-changed' signal when navigating a combo box with keyboard
     4        https://bugs.webkit.org/show_bug.cgi?id=133512
     5
     6        Reviewed by Chris Fleizach.
     7
     8        Added support for connecting to AtkSelection's 'selection-changed'
     9        signal, and print it out as AXSelectedChildrenChanged in the tests.
     10
     11        Also removed some dead code, that became useless after r169487.
     12
     13        * WebKitTestRunner/InjectedBundle/atk/AccessibilityNotificationHandlerAtk.cpp:
     14        (WTR::AccessibilityNotificationHandler::connectAccessibilityCallbacks): Updated.
     15
    1162014-06-15  Ryuan Choi  <ryuan.choi@samsung.com>
    217

  • trunk/Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityNotificationHandlerAtk.cpp

    r169487 r170008  
    6464
    6565    GSignalQuery signalQuery;
    66     GUniquePtr<char> signalName;
    67     GUniquePtr<char> signalValue;
    6866    const char* notificationName = nullptr;
    6967    Vector<JSValueRef> extraArgs;
    … …  
    7270
    7371    if (!g_strcmp0(signalQuery.signal_name, "state-change")) {
    74         signalName.reset(g_strdup_printf("state-change:%s", g_value_get_string(&paramValues[1])));
    75         signalValue.reset(g_strdup_printf("%d", g_value_get_boolean(&paramValues[2])));
    7672        if (!g_strcmp0(g_value_get_string(&paramValues[1]), "checked"))
    7773            notificationName = "CheckedStateChanged";
    … …  
    7975            notificationName = "AXInvalidStatusChanged";
    8076    } else if (!g_strcmp0(signalQuery.signal_name, "focus-event")) {
    81         signalName.reset(g_strdup("focus-event"));
    82         signalValue.reset(g_strdup_printf("%d", g_value_get_boolean(&paramValues[1])));
    8377        if (g_value_get_boolean(&paramValues[1]))
    8478            notificationName = "AXFocusedUIElementChanged";
     79    } else if (!g_strcmp0(signalQuery.signal_name, "selection-changed")) {
     80        notificationName = "AXSelectedChildrenChanged";
    8581    } else if (!g_strcmp0(signalQuery.signal_name, "children-changed")) {
    8682        const gchar* childrenChangedDetail = g_quark_to_string(signalHint->detail);
    87         signalName.reset(g_strdup_printf("children-changed:%s", childrenChangedDetail));
    88         signalValue.reset(g_strdup_printf("%d", g_value_get_uint(&paramValues[1])));
    8983        notificationName = !g_strcmp0(childrenChangedDetail, "add") ? "AXChildrenAdded" : "AXChildrenRemoved";
    9084    } else if (!g_strcmp0(signalQuery.signal_name, "property-change")) {
    91         signalName.reset(g_strdup_printf("property-change:%s", g_quark_to_string(signalHint->detail)));
    9285        if (!g_strcmp0(g_quark_to_string(signalHint->detail), "accessible-value"))
    9386            notificationName = "AXValueChanged";
    … …  
    9689    else if (!g_strcmp0(signalQuery.signal_name, "text-caret-moved")) {
    9790        notificationName = "AXTextCaretMoved";
    98         signalName.reset(g_strdup(signalQuery.signal_name));
    99         signalValue.reset(g_strdup_printf("%d", g_value_get_int(&paramValues[1])));
     91        GUniquePtr<char> signalValue(g_strdup_printf("%d", g_value_get_int(&paramValues[1])));
    10092        JSRetainPtr<JSStringRef> jsSignalValue(Adopt, JSStringCreateWithUTF8CString(signalValue.get()));
    10193        extraArgs.append(JSValueMakeString(jsContext, jsSignalValue.get()));
    102     } else
    103         signalName.reset(g_strdup(signalQuery.signal_name));
     94    }
    10495
    10596    if (!jsContext)
    … …  
    226217        "ATK:AtkObject:visible-data-changed",
    227218        "ATK:AtkDocument:load-complete",
     219        "ATK:AtkSelection:selection-changed",
    228220        "ATK:AtkText:text-caret-moved",
    229221        0
Note: See TracChangeset for help on using the changeset viewer.