233004 2021-11-11 10:25:55 -0800 AX: Add missing AXID variant to AXPropertyValueVariant 2021-11-17 08:11:08 -0800 1 1 1 Unclassified WebKit Accessibility WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=233248 InRadar P2 Normal --- 1 tyler_w tyler_w aboxhall andresg_22 apinheiro cfleizach cgarcia dmazzoni ews-watchlist jcraig jdiggs samuel_white webkit-bug-importer oldest_to_newest 1814097 0 tyler_w 2021-11-11 10:25:55 -0800 In this method: void AXIsolatedObject::setObjectProperty(AXPropertyName propertyName, AXCoreObject* object) { if (object) setProperty(propertyName, object->objectID()); else setProperty(propertyName, nullptr, true); } We set properties with AXID values. However, there is no AXID variant in AXPropertyValueVariant, so none of these property lookups ever returned values. 1814100 1 webkit-bug-importer 2021-11-11 10:26:09 -0800 <rdar://problem/85305185> 1814104 2 443970 tyler_w 2021-11-11 10:27:58 -0800 Created attachment 443970 Patch 1814278 3 ews-feeder 2021-11-11 15:50:28 -0800 Committed r285686 (244161@main): <https://commits.webkit.org/244161@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 443970. 1815853 4 cgarcia 2021-11-17 00:51:24 -0800 hmm, this broke the build here with isolated tree enabled, I don't know if it's because of GCC: Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:482:51: error: cannot convert ‘WebCore::AXID’ {aka ‘long unsigned int’} to ‘WebCore::AXPropertyValueVariant&&’ {aka ‘std::variant<std::nullptr_t, long unsigned int, WTF::String, bool, int, unsigned int, double, float, long unsigned int, WebCore::Color, WTF::URL, WebCore::LayoutRect, WebCore::FloatRect, PAL::SessionID, WebCore::IntPoint, WTF::OptionSet<WebCore::SpeakAs>, std::pair<unsigned int, unsigned int>, WTF::Vector<WebCore::AccessibilityText, 0, WTF::CrashOnOverflow, 16, WTF::FastMalloc>, WTF::Vector<long unsigned int, 0, WTF::CrashOnOverflow, 16, WTF::FastMalloc>, WTF::Vector<std::pair<long unsigned int, long unsigned int>, 0, WTF::CrashOnOverflow, 16, WTF::FastMalloc>, WTF::Vector<WTF::String, 0, WTF::CrashOnOverflow, 16, WTF::FastMalloc>, WebCore::Path, WTF::OptionSet<WebCore::AXAncestorFlag> >&&’} 482 | setProperty(propertyName, object->objectID()); | ~~~~~~~~~~~~~~~~^~ | | | WebCore::AXID {aka long unsigned int} Maybe the problem is that we now have long unsigned int twice AXID and uint64_t. 1815943 5 tyler_w 2021-11-17 08:11:08 -0800 Sorry for causing more GTK breakage, Carlos. Indeed, it must be a difference in the way GCC and Clang handle size_t in this case. Just to reconfirm the results of this patch with Clang + Mac, I ran the tests one more time (run-webkit-tests --no-retry --release --accessibility-isolated-tree accessibility/): My main is currently at revision: https://github.com/WebKit/WebKit/commit/ce9ed8749594cb74cb8b154c56c91a8fe0bac858 Main with r285686: 161 text failures, 3 crashes, 8 timeouts Main without r285686: 197 text failures, 1 crash, 11 timeouts I also ran the tests with your latest AXID ObjectIdentifier patch (https://bugs.webkit.org/attachment.cgi?id=444509&action=prettypatch) + main and got: 160 text failures, 2 crashes, 9 timeouts Which seems good to me. Thanks for fixing this in https://bugs.webkit.org/show_bug.cgi?id=233248! 443970 2021-11-11 10:27:58 -0800 2021-11-11 16:51:50 -0800 Patch bug-233004-20211111122757.patch text/plain 2354 tyler_w U3VidmVyc2lvbiBSZXZpc2lvbjogMjg1NjMzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMTFmYzMxYjkyNWY0MDI4 ODM0OGZiNmQyYzk1YWUwMTJkYTJkOTc0My4uYzc2ODEyMThhMDhjNDQ3Yzg0Y2ZlMjgwYjVjOGU5 NGIyMDRiMDgyOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDIxLTExLTExICBUeWxl ciBXaWxjb2NrICA8dHlsZXJfd0BhcHBsZS5jb20+CisKKyAgICAgICAgQVg6IEFkZCBtaXNzaW5n IEFYSUQgdmFyaWFudCB0byBBWFByb3BlcnR5VmFsdWVWYXJpYW50CisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMzMwMDQKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBJbiBBWElzb2xhdGVkT2JqZWN0OjpzZXRP YmplY3RQcm9wZXJ0eSwgd2Ugc2V0IHByb3BlcnR5IHZhbHVlcyB0byBiZQorICAgICAgICBBWElE cy4gSG93ZXZlciwgdGhlcmUgaXMgbm8gQVhJRCB2YXJpYW50IGluIEFYUHJvcGVydHlWYWx1ZVZh cmlhbnQsCisgICAgICAgIHNvIG5vbmUgb2YgdGhlc2UgcHJvcGVydHkgbG9va3VwcyBldmVyIHJl dHVybmVkIHZhbHVlcy4KKworICAgICAgICBUaGlzIHBhdGNoIGFkZHMgQVhJRCBhcyBhIHZhcmlh bnQgdG8gQVhQcm9wZXJ0eVZhbHVlVmFyaWFudCwgZml4aW5nCisgICAgICAgIH4xNSB0ZXh0LW9u bHkgZmFpbHVyZXMuCisKKyAgICAgICAgKiBhY2Nlc3NpYmlsaXR5L2lzb2xhdGVkdHJlZS9BWElz b2xhdGVkVHJlZS5oOgorICAgICAgICBBZGQgQVhJRCBhcyBhIHZhcmlhbnQgdG8gQVhQcm9wZXJ0 eVZhbHVlVmFyaWFudC4KKwogMjAyMS0xMS0xMSAgVHlsZXIgV2lsY29jayAgPHR5bGVyX3dAYXBw bGUuY29tPgogCiAgICAgICAgIEFYOiBJbXBsZW1lbnQgQVhDb3JlT2JqZWN0Ojp2YWx1ZSBmb3Ig QVhJc29sYXRlZE9iamVjdApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0 eS9pc29sYXRlZHRyZWUvQVhJc29sYXRlZFRyZWUuaCBiL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2li aWxpdHkvaXNvbGF0ZWR0cmVlL0FYSXNvbGF0ZWRUcmVlLmgKaW5kZXggZjcyNWY1N2I5MzNhZTEy ODg2ZDEzMWU5OWI4ZjY2MThlNzBjMTJhMi4uMTZiZDQ2ZjQ5OTYxZTQ0ZmE0ZmQ3NTdjYjFiMzEw Y2FjNzE5ZjhhNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9pc29s YXRlZHRyZWUvQVhJc29sYXRlZFRyZWUuaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmls aXR5L2lzb2xhdGVkdHJlZS9BWElzb2xhdGVkVHJlZS5oCkBAIC0zMTksNyArMzE5LDcgQEAgZW51 bSBjbGFzcyBBWFByb3BlcnR5TmFtZSA6IHVpbnQxNl90IHsKICAgICBXZWJBcmVhLAogfTsKIAot dXNpbmcgQVhQcm9wZXJ0eVZhbHVlVmFyaWFudCA9IHN0ZDo6dmFyaWFudDxzdGQ6Om51bGxwdHJf dCwgU3RyaW5nLCBib29sLCBpbnQsIHVuc2lnbmVkLCBkb3VibGUsIGZsb2F0LCB1aW50NjRfdCwg Q29sb3IsIFVSTCwgTGF5b3V0UmVjdCwgRmxvYXRSZWN0LCBQQUw6OlNlc3Npb25JRCwgSW50UG9p bnQsIE9wdGlvblNldDxTcGVha0FzPiwgc3RkOjpwYWlyPHVuc2lnbmVkLCB1bnNpZ25lZD4sIFZl Y3RvcjxBY2Nlc3NpYmlsaXR5VGV4dD4sIFZlY3RvcjxBWElEPiwgVmVjdG9yPHN0ZDo6cGFpcjxB WElELCBBWElEPj4sIFZlY3RvcjxTdHJpbmc+LCBQYXRoLCBPcHRpb25TZXQ8QVhBbmNlc3RvckZs YWc+PjsKK3VzaW5nIEFYUHJvcGVydHlWYWx1ZVZhcmlhbnQgPSBzdGQ6OnZhcmlhbnQ8c3RkOjpu dWxscHRyX3QsIEFYSUQsIFN0cmluZywgYm9vbCwgaW50LCB1bnNpZ25lZCwgZG91YmxlLCBmbG9h dCwgdWludDY0X3QsIENvbG9yLCBVUkwsIExheW91dFJlY3QsIEZsb2F0UmVjdCwgUEFMOjpTZXNz aW9uSUQsIEludFBvaW50LCBPcHRpb25TZXQ8U3BlYWtBcz4sIHN0ZDo6cGFpcjx1bnNpZ25lZCwg dW5zaWduZWQ+LCBWZWN0b3I8QWNjZXNzaWJpbGl0eVRleHQ+LCBWZWN0b3I8QVhJRD4sIFZlY3Rv cjxzdGQ6OnBhaXI8QVhJRCwgQVhJRD4+LCBWZWN0b3I8U3RyaW5nPiwgUGF0aCwgT3B0aW9uU2V0 PEFYQW5jZXN0b3JGbGFnPj47CiB1c2luZyBBWFByb3BlcnR5TWFwID0gSGFzaE1hcDxBWFByb3Bl cnR5TmFtZSwgQVhQcm9wZXJ0eVZhbHVlVmFyaWFudCwgSW50SGFzaDxBWFByb3BlcnR5TmFtZT4s IFdURjo6U3Ryb25nRW51bUhhc2hUcmFpdHM8QVhQcm9wZXJ0eU5hbWU+PjsKIAogc3RydWN0IEFY UHJvcGVydHlDaGFuZ2Ugewo=