163088 2016-10-06 14:30:11 -0700 Web Inspector: AXI: focused/focusable state should be based on Accessibility Object instead of Element 2016-10-18 16:07:00 -0700 1 1 1 Unclassified WebKit Accessibility Safari 10 All All RESOLVED FIXED InRadar P2 Normal --- 1 aaron_chu webkit-unassigned aaron_chu commit-queue webkit-bug-importer oldest_to_newest 1237357 0 aaron_chu 2016-10-06 14:30:11 -0700 The current focus implementation uses a DOM focusable check instead of and accessibility focusable check. 1237358 1 aaron_chu 2016-10-06 14:33:09 -0700 <rdar://problem/16421985> 1237659 2 290941 aaron_chu 2016-10-07 10:41:24 -0700 Created attachment 290941 Patch 1237729 3 290941 darin 2016-10-07 12:21:11 -0700 Comment on attachment 290941 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290941&action=review > Source/WebCore/ChangeLog:13 > + Covered by existing tests: > + LayoutTests/inspector/dom/getAccessibilityPropertiesForNode.html Maybe the code is exercised by that test, but does that test show the bug? If it did, then there would need to be a test result update in this patch. 1237740 4 290941 aaron_chu 2016-10-07 12:31:02 -0700 Comment on attachment 290941 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290941&action=review >> Source/WebCore/ChangeLog:13 >> + LayoutTests/inspector/dom/getAccessibilityPropertiesForNode.html > > Maybe the code is exercised by that test, but does that test show the bug? If it did, then there would need to be a test result update in this patch. I ran the test with the new code and the result was consistant with the result. Would that suffice? 1238942 5 290941 joepeck 2016-10-11 12:07:00 -0700 Comment on attachment 290941 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290941&action=review >>> Source/WebCore/ChangeLog:13 >>> + LayoutTests/inspector/dom/getAccessibilityPropertiesForNode.html >> >> Maybe the code is exercised by that test, but does that test show the bug? If it did, then there would need to be a test result update in this patch. > > I ran the test with the new code and the result was consistant with the result. Would that suffice? Darin has a point. If this change is fixing an issue, then there should be a test that shows what changed. The existing test didn't change its results, so whatever this patch is fixing is not included in that test! Ideally you would update the test to cover the case that this patch fixes. That will ensure the issue fixed by this patch doesn't regress in the future. 1241655 6 290941 joepeck 2016-10-18 15:43:34 -0700 Comment on attachment 290941 Patch From Aaron via email: > Technically there isn’t an issue, noting was breaking. I think James > original concern was that the isFocusable was not “authenticatic” > because it was not coming from an AXObject while we are trying to > show the related properties in the AX Inspector. Sounds good to land. 1241662 7 290941 commit-queue 2016-10-18 16:06:56 -0700 Comment on attachment 290941 Patch Clearing flags on attachment: 290941 Committed r207496: <http://trac.webkit.org/changeset/207496> 1241663 8 commit-queue 2016-10-18 16:07:00 -0700 All reviewed patches have been landed. Closing bug. 290941 2016-10-07 10:41:24 -0700 2016-10-18 16:06:56 -0700 Patch bug-163088-20161007103809.patch text/plain 1822 aaron_chu U3VidmVyc2lvbiBSZXZpc2lvbjogMjA2ODg0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggODcyNzY1ZjAyZmZkMzA0 NzQwM2VlYjk1N2MxM2I1MzU3NzFhMzA4Yi4uZjU2NjllZTg3ZjZkNTdlNWM1YTY2NWM1OTAzOWJi OThiNjlhNmFhZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDE2LTEwLTA3ICBBYXJv biBDaHUgIDxhYXJvbl9jaHVAYXBwbGUuY29tPgorCisgICAgICAgIFdlYiBJbnNwZWN0b3I6IEFY STogZm9jdXNlZC9mb2N1c2FibGUgc3RhdGUgc2hvdWxkIGJlIGJhc2VkIG9uIEFjY2Vzc2liaWxp dHkgT2JqZWN0IGluc3RlYWQgb2YgRWxlbWVudAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTYzMDg4CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8xNjQy MTk4NT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBD aGFuZ2VkIGNvZGUgaW4gSW5zcGVjdERPTUFnZW50IHNvIHRoYXQgaXQgZGV0ZXJtaW5lcyB0aGUg Zm9jdXNhYmlsaXR5IG9mIGEgTm9kZSBiYXNlZAorICAgICAgICBvbiB0aGUgQWNjZXNzaWJpbGl0 eU5vZGVPYmplY3QgYW5kIG5vdCB0aGUgRWxlbWVudCBjbGFzcy4KKworICAgICAgICBDb3ZlcmVk IGJ5IGV4aXN0aW5nIHRlc3RzOiAKKyAgICAgICAgTGF5b3V0VGVzdHMvaW5zcGVjdG9yL2RvbS9n ZXRBY2Nlc3NpYmlsaXR5UHJvcGVydGllc0Zvck5vZGUuaHRtbAorCisgICAgICAgICogaW5zcGVj dG9yL0luc3BlY3RvckRPTUFnZW50LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Okluc3BlY3RvckRP TUFnZW50OjpidWlsZE9iamVjdEZvckFjY2Vzc2liaWxpdHlQcm9wZXJ0aWVzKToKKwogMjAxNi0x MC0wNiAgWmFsYW4gQnVqdGFzICA8emFsYW5AYXBwbGUuY29tPgogCiAgICAgICAgIEFkZCBiYWNr IEFTU0VSVCghbmVlZHNMYXlvdXQpIHRvIFJlbmRlclRhYmxlU2VjdGlvbiB3aGljaCBpcyBub3cg dmFsaWQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2luc3BlY3Rvci9JbnNwZWN0b3JET01B Z2VudC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvSW5zcGVjdG9yRE9NQWdlbnQuY3Bw CmluZGV4IDE2N2U1ZmJlNzhmNzI5MjcyOTBlODgzOTUwMDk2ZDcwOTU2MmMyNTQuLjU5YmM1YTM1 NWE0ZmViYzM4OGYyMTc3YmRjMzhmMWM4ZThiZDY2MzEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL2luc3BlY3Rvci9JbnNwZWN0b3JET01BZ2VudC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUv aW5zcGVjdG9yL0luc3BlY3RvckRPTUFnZW50LmNwcApAQCAtMTY3NSw3ICsxNjc1LDcgQEAgUmVm UHRyPEluc3BlY3Rvcjo6UHJvdG9jb2w6OkRPTTo6QWNjZXNzaWJpbGl0eVByb3BlcnRpZXM+IElu c3BlY3RvckRPTUFnZW50OjpidWkKICAgICAgICAgICAgIH0KICAgICAgICAgICAgIAogICAgICAg ICAgICAgaWYgKGlzPEVsZW1lbnQ+KCpub2RlKSkgewotICAgICAgICAgICAgICAgIHN1cHBvcnRz Rm9jdXNlZCA9IGRvd25jYXN0PEVsZW1lbnQ+KCpub2RlKS5pc0ZvY3VzYWJsZSgpOworICAgICAg ICAgICAgICAgIHN1cHBvcnRzRm9jdXNlZCA9IGF4T2JqZWN0LT5jYW5TZXRGb2N1c0F0dHJpYnV0 ZSgpOwogICAgICAgICAgICAgICAgIGlmIChzdXBwb3J0c0ZvY3VzZWQpCiAgICAgICAgICAgICAg ICAgICAgIGZvY3VzZWQgPSBheE9iamVjdC0+aXNGb2N1c2VkKCk7CiAgICAgICAgICAgICB9Cg==