106349 2013-01-08 10:59:35 -0800 AX: native popup buttons should not use textUnderElement for their title 2013-01-09 07:57:36 -0800 1 1 1 Unclassified WebKit Accessibility 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 cfleizach cfleizach aboxhall apinheiro dglazkov dmazzoni jdiggs noel.gordon rniwa webkit-ews webkit.review.bot oldest_to_newest 802390 0 cfleizach 2013-01-08 10:59:35 -0800 Native pop up buttons (<select>) should not use textUnderElement for their visibleText/title. That's because that would cause the title to be the value of the pop up button. We want to communicate that info through stringValue(), not the title 802418 1 181714 cfleizach 2013-01-08 11:17:41 -0800 Created attachment 181714 patch 802426 2 181714 dmazzoni 2013-01-08 11:23:35 -0800 Comment on attachment 181714 patch Unofficial review. View in context: https://bugs.webkit.org/attachment.cgi?id=181714&action=review > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1196 > + if (renderer() && renderer()->isMenuList()) This should work for elements without a renderer (like in canvas fallback content, or in a region of the page that's hidden from sighted users but has aria-hidden=false). Instead of checking renderer()->isMenuList(), check if it's a select element and if it has multiple or not. There's already similar code in this file. If there are cases where a rendered popup button is different than an unrendered one, you could override it in AccessibilityRenderObject - but I don't think you need to in this case. 802427 3 181714 webkit-ews 2013-01-08 11:24:26 -0800 Comment on attachment 181714 patch Attachment 181714 did not pass qt-ews (qt): Output: http://queues.webkit.org/results/15775013 802428 4 181714 webkit-ews 2013-01-08 11:26:28 -0800 Comment on attachment 181714 patch Attachment 181714 did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/15763182 802429 5 cfleizach 2013-01-08 11:26:43 -0800 (In reply to comment #2) > (From update of attachment 181714 [details]) > Unofficial review. > > View in context: https://bugs.webkit.org/attachment.cgi?id=181714&action=review > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1196 > > + if (renderer() && renderer()->isMenuList()) > > This should work for elements without a renderer (like in canvas fallback content, or in a region of the page that's hidden from sighted users but has aria-hidden=false). Instead of checking renderer()->isMenuList(), check if it's a select element and if it has multiple or not. There's already similar code in this file. i think we want this to apply for multiple and non-multiple. what do you think > > If there are cases where a rendered popup button is different than an unrendered one, you could override it in AccessibilityRenderObject - but I don't think you need to in this case. 802433 6 dmazzoni 2013-01-08 11:30:35 -0800 (In reply to comment #5) > > This should work for elements without a renderer (like in canvas fallback content, or in a region of the page that's hidden from sighted users but has aria-hidden=false). Instead of checking renderer()->isMenuList(), check if it's a select element and if it has multiple or not. There's already similar code in this file. > > i think we want this to apply for multiple and non-multiple. what do you think I think you're right, but then I'd like both to be covered by tests. 802435 7 cfleizach 2013-01-08 11:32:32 -0800 (In reply to comment #6) > (In reply to comment #5) > > > This should work for elements without a renderer (like in canvas fallback content, or in a region of the page that's hidden from sighted users but has aria-hidden=false). Instead of checking renderer()->isMenuList(), check if it's a select element and if it has multiple or not. There's already similar code in this file. > > > > i think we want this to apply for multiple and non-multiple. what do you think > > I think you're right, but then I'd like both to be covered by tests. It looks like they are <select id="combobox1"><option>1<option selected>2</select> <select multiple id="listbox1"><option>1<option selected>2</select> 802438 8 181715 cfleizach 2013-01-08 11:33:47 -0800 Created attachment 181715 patch 802441 9 dmazzoni 2013-01-08 11:36:28 -0800 Looks fine to me. 802456 10 cfleizach 2013-01-08 11:54:25 -0800 http://trac.webkit.org/changeset/139091 802637 11 dglazkov 2013-01-08 14:25:26 -0800 (In reply to comment #10) > http://trac.webkit.org/changeset/139091 The test is still failing across Chromium. What gives? http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=accessibility%2Fcanvas-fallback-content-2.html 802644 12 rniwa 2013-01-08 14:26:37 -0800 (In reply to comment #11) > (In reply to comment #10) > > http://trac.webkit.org/changeset/139091 > > The test is still failing across Chromium. What gives? > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=accessibility%2Fcanvas-fallback-content-2.html Odd. Maybe Dominic would know why? 802881 13 noel.gordon 2013-01-08 18:44:42 -0800 The failure is due to changes in ax title rendering (aka this bug and related). combobox1 PASS document.activeElement == element1 is true1 combobox2 PASS document.activeElement == element2 is true PASS axElement2.role is axElement1.role PASS axElement2.roleDescription is axElement1.roleDescription FAIL axElement2.title should be AXTitle: 2. Was AXTitle: . PASS axElement2.description is axElement1.description PASS axElement2.helpText is axElement1.helpText PASS axElement2.stringValue is axElement1.stringValue PASS axElement2.isEnabled is axElement1.isEnabled PASS axElement2.isRequired is axElement1.isRequired PASS axElement2.isChecked is axElement1.isChecked PASS axElement2.intValue is axElement1.intValue PASS axElement2.minValue is axElement1.minValue PASS axElement2.maxValue is axElement1.maxValue 802883 14 noel.gordon 2013-01-08 18:45:51 -0800 And the green cr-linux bubble was trying to warn you about it. Regressions: Unexpected text-only failures (1) accessibility/canvas-fallback-content-2.html [ Failure ] 803105 15 181867 cfleizach 2013-01-09 00:18:53 -0800 Created attachment 181867 patch for chromium layout test fix 803120 16 noel.gordon 2013-01-09 01:07:21 -0800 lgtm: accessibility/canvas-fallback-content-2.html passes for me on chrome mac with this change. 803349 17 cfleizach 2013-01-09 07:57:36 -0800 Chromium fix http://trac.webkit.org/changeset/139195 181714 2013-01-08 11:17:41 -0800 2013-01-08 11:33:47 -0800 patch patch text/plain 1766 cfleizach SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEzOTA4MikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDEzLTAxLTA4ICBDaHJpcyBG bGVpemFjaCAgPGNmbGVpemFjaEBhcHBsZS5jb20+CisKKyAgICAgICAgQVg6IG5hdGl2ZSBwb3B1 cCBidXR0b25zIHNob3VsZCBub3QgdXNlIHRleHRVbmRlckVsZW1lbnQgZm9yIHRoZWlyIHRpdGxl CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDYzNDkK KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBOYXRpdmUg cG9wdXAgYnV0dG9ucyAoPHNlbGVjdD4gZWxlbWVudHMpIHdlcmUgY2FsY3VsYXRpbmcgaXRzIHRp dGxlIGJhc2VkIG9uIHRoZSB0ZXh0CisgICAgICAgIHVuZGVyIHRoZSBlbGVtZW50LiBXaGVuIEJ1 ZyAxMDM3OTQgaW50cm9kdWNlZCBhIG1vcmUgY29tcGxldGUgd2F5IG9mIGdldHRpbmcgcmVuZGVy IHRleHQsCisgICAgICAgIGl0IGNhdXNlcyBwb3B1cCBidXR0b25zIHRvIHN0YXJ0IHJldHVybmlu ZyBhIHRpdGxlLgorICAgICAgICBUaGUgZml4IGlzIHRoYXQgd2Ugc2hvdWxkIG5vdCBiZSBjYWxj dWxhdGluZyBhIHRpdGxlIGZvciBhIG5hdGl2ZSBwb3B1cCBidXR0b24gZnJvbSBpdHMgY2hpbGRy ZW4uCisKKyAgICAgICAgRXhpc3RpbmcgdGVzdHMgY292ZXIgdGhpcyBmdW5jdGlvbmFsaXR5LiBU aGlzIHdpbGwgZml4IGEgZmFpbGluZyB0ZXN0LgorCisgICAgICAgICogYWNjZXNzaWJpbGl0eS9B Y2Nlc3NpYmlsaXR5Tm9kZU9iamVjdC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpBY2Nlc3NpYmls aXR5Tm9kZU9iamVjdDo6dmlzaWJsZVRleHQpOgorCiAyMDEzLTAxLTA4ICBaYW4gRG9iZXJzZWsg IDx6YW5kb2JlcnNla0BnbWFpbC5jb20+CiAKICAgICAgICAgW0dUS10gQnVpbGQgZmFpbHVyZXMg d2hlbiBidWlsZGluZyB3aXRoIHB5dGhvbiAzLjMKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL2FjY2Vz c2liaWxpdHkvQWNjZXNzaWJpbGl0eU5vZGVPYmplY3QuY3BwCj09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJj ZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvQWNjZXNzaWJpbGl0eU5vZGVPYmplY3QuY3BwCShyZXZp c2lvbiAxMzkwMDEpCisrKyBTb3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L0FjY2Vzc2liaWxp dHlOb2RlT2JqZWN0LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTE5Miw2ICsxMTkyLDkgQEAKICAg ICAKICAgICBzd2l0Y2ggKHJvbGVWYWx1ZSgpKSB7CiAgICAgY2FzZSBQb3BVcEJ1dHRvblJvbGU6 CisgICAgICAgIC8vIE5hdGl2ZSBwb3B1cCBidXR0b25zIHNob3VsZCBub3QgdXNlIHRoZWlyIGJ1 dHRvbiBjaGlsZHJlbidzIHRleHQgYXMgYSB0aXRsZS4gVGhhdCB2YWx1ZSBpcyByZXRyaWV2ZWQg dGhyb3VnaCBzdHJpbmdWYWx1ZSgpLgorICAgICAgICBpZiAocmVuZGVyZXIoKSAmJiByZW5kZXJl cigpLT5pc01lbnVMaXN0KCkpCisgICAgICAgICAgICBicmVhazsKICAgICBjYXNlIEJ1dHRvblJv bGU6CiAgICAgY2FzZSBUb2dnbGVCdXR0b25Sb2xlOgogICAgIGNhc2UgQ2hlY2tCb3hSb2xlOgo= 181715 2013-01-08 11:33:47 -0800 2013-01-08 11:53:07 -0800 patch patch text/plain 1755 cfleizach SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEzOTA4MikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDEzLTAxLTA4ICBDaHJpcyBG bGVpemFjaCAgPGNmbGVpemFjaEBhcHBsZS5jb20+CisKKyAgICAgICAgQVg6IG5hdGl2ZSBwb3B1 cCBidXR0b25zIHNob3VsZCBub3QgdXNlIHRleHRVbmRlckVsZW1lbnQgZm9yIHRoZWlyIHRpdGxl CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDYzNDkK KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBOYXRpdmUg cG9wdXAgYnV0dG9ucyAoPHNlbGVjdD4gZWxlbWVudHMpIHdlcmUgY2FsY3VsYXRpbmcgaXRzIHRp dGxlIGJhc2VkIG9uIHRoZSB0ZXh0CisgICAgICAgIHVuZGVyIHRoZSBlbGVtZW50LiBXaGVuIEJ1 ZyAxMDM3OTQgaW50cm9kdWNlZCBhIG1vcmUgY29tcGxldGUgd2F5IG9mIGdldHRpbmcgcmVuZGVy IHRleHQsCisgICAgICAgIGl0IGNhdXNlcyBwb3B1cCBidXR0b25zIHRvIHN0YXJ0IHJldHVybmlu ZyBhIHRpdGxlLgorICAgICAgICBUaGUgZml4IGlzIHRoYXQgd2Ugc2hvdWxkIG5vdCBiZSBjYWxj dWxhdGluZyBhIHRpdGxlIGZvciBhIG5hdGl2ZSBwb3B1cCBidXR0b24gZnJvbSBpdHMgY2hpbGRy ZW4uCisKKyAgICAgICAgRXhpc3RpbmcgdGVzdHMgY292ZXIgdGhpcyBmdW5jdGlvbmFsaXR5LiBU aGlzIHdpbGwgZml4IGEgZmFpbGluZyB0ZXN0LgorCisgICAgICAgICogYWNjZXNzaWJpbGl0eS9B Y2Nlc3NpYmlsaXR5Tm9kZU9iamVjdC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpBY2Nlc3NpYmls aXR5Tm9kZU9iamVjdDo6dmlzaWJsZVRleHQpOgorCiAyMDEzLTAxLTA4ICBaYW4gRG9iZXJzZWsg IDx6YW5kb2JlcnNla0BnbWFpbC5jb20+CiAKICAgICAgICAgW0dUS10gQnVpbGQgZmFpbHVyZXMg d2hlbiBidWlsZGluZyB3aXRoIHB5dGhvbiAzLjMKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL2FjY2Vz c2liaWxpdHkvQWNjZXNzaWJpbGl0eU5vZGVPYmplY3QuY3BwCj09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJj ZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvQWNjZXNzaWJpbGl0eU5vZGVPYmplY3QuY3BwCShyZXZp c2lvbiAxMzkwMDEpCisrKyBTb3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L0FjY2Vzc2liaWxp dHlOb2RlT2JqZWN0LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTE5Miw2ICsxMTkyLDkgQEAKICAg ICAKICAgICBzd2l0Y2ggKHJvbGVWYWx1ZSgpKSB7CiAgICAgY2FzZSBQb3BVcEJ1dHRvblJvbGU6 CisgICAgICAgIC8vIE5hdGl2ZSBwb3B1cCBidXR0b25zIHNob3VsZCBub3QgdXNlIHRoZWlyIGJ1 dHRvbiBjaGlsZHJlbidzIHRleHQgYXMgYSB0aXRsZS4gVGhhdCB2YWx1ZSBpcyByZXRyaWV2ZWQg dGhyb3VnaCBzdHJpbmdWYWx1ZSgpLgorICAgICAgICBpZiAobm9kZS0+aGFzVGFnTmFtZShzZWxl Y3RUYWcpKQorICAgICAgICAgICAgYnJlYWs7CiAgICAgY2FzZSBCdXR0b25Sb2xlOgogICAgIGNh c2UgVG9nZ2xlQnV0dG9uUm9sZToKICAgICBjYXNlIENoZWNrQm94Um9sZToK 181867 2013-01-09 00:18:53 -0800 2013-01-09 01:10:32 -0800 patch for chromium layout test fix patch text/plain 1608 cfleizach SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEzOTE2OSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDEzLTAxLTA5ICBDaHJpcyBG bGVpemFjaCAgPGNmbGVpemFjaEBhcHBsZS5jb20+CisKKyAgICAgICAgQVg6IG5hdGl2ZSBwb3B1 cCBidXR0b25zIHNob3VsZCBub3QgdXNlIHRleHRVbmRlckVsZW1lbnQgZm9yIHRoZWlyIHRpdGxl CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDYzNDkK KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBDaHJvbWl1 bSByZWxpZXMgb24gdGhlIG9sZGVyIHRpdGxlKCkgbWV0aG9kIHRvIHJldHVybiBpdHMgdGl0bGUu IEFzIGEgcmVzdWx0LCB3ZSBhbHNvCisgICAgICAgIG5lZWQgdG8gaGFuZGxlIHRoZSBjYXNlIG9m IDxzZWxlY3Q+IGVsZW1lbnQgcG9wIHVwIGJ1dHRvbnMgcmV0dXJuaW5nIHRoZSB0ZXh0VW5kZXJF bGVtZW50KCkuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLiBGaXggZXhpc3RpbmcgYnJlYWsuCisK KyAgICAgICAgKiBhY2Nlc3NpYmlsaXR5L0FjY2Vzc2liaWxpdHlOb2RlT2JqZWN0LmNwcDoKKyAg ICAgICAgKFdlYkNvcmU6OkFjY2Vzc2liaWxpdHlOb2RlT2JqZWN0Ojp0aXRsZSk6CisKIDIwMTMt MDEtMDggIEFycGl0YSBCYWh1Z3VuYSAgPGFycGl0YWJhaHVndW5hQGdtYWlsLmNvbT4KIAogICAg ICAgICBDYXJldCBpcyBpbmNvcnJlY3RseSBwYWludGVkIGZvciBhIGNvbnRlbnRlZGl0YWJsZSA8 ZGl2PiBjb250YWluaW5nIGEgPGJyPiBpbiB2ZXJ0aWNhbCB3cml0aW5nIG1vZGUKSW5kZXg6IFNv dXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvQWNjZXNzaWJpbGl0eU5vZGVPYmplY3QuY3BwCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvQWNjZXNzaWJpbGl0eU5v ZGVPYmplY3QuY3BwCShyZXZpc2lvbiAxMzkxMjQpCisrKyBTb3VyY2UvV2ViQ29yZS9hY2Nlc3Np YmlsaXR5L0FjY2Vzc2liaWxpdHlOb2RlT2JqZWN0LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTUx NSw2ICsxNTE1LDkgQEAKIAogICAgIHN3aXRjaCAocm9sZVZhbHVlKCkpIHsKICAgICBjYXNlIFBv cFVwQnV0dG9uUm9sZToKKyAgICAgICAgLy8gTmF0aXZlIHBvcHVwIGJ1dHRvbnMgc2hvdWxkIG5v dCB1c2UgdGhlaXIgYnV0dG9uIGNoaWxkcmVuJ3MgdGV4dCBhcyBhIHRpdGxlLiBUaGF0IHZhbHVl IGlzIHJldHJpZXZlZCB0aHJvdWdoIHN0cmluZ1ZhbHVlKCkuCisgICAgICAgIGlmIChub2RlLT5o YXNUYWdOYW1lKHNlbGVjdFRhZykpCisgICAgICAgICAgICByZXR1cm4gU3RyaW5nKCk7CiAgICAg Y2FzZSBCdXR0b25Sb2xlOgogICAgIGNhc2UgVG9nZ2xlQnV0dG9uUm9sZToKICAgICBjYXNlIENo ZWNrQm94Um9sZToK