Currently failing in ITM.
<rdar://problem/102528770>
Created attachment 463610 [details] Patch
Comment on attachment 463610 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=463610&action=review > LayoutTests/accessibility/mac/html-with-aria-label.html:20 > + html.setAttribute("aria-label", "New Title"); I see this and some other places in this patch are using some implicit object-by-id JS syntax. I think this not recommended: https://html.spec.whatwg.org/#named-access-on-the-window-object > As a general rule, relying on this will lead to brittle code. Which IDs end up mapping to this API can vary over time, as new features are added to the web platform, for example. Instead of this, use document.getElementById() or document.querySelector(). > LayoutTests/accessibility/mac/html-with-aria-label.html:22 > + output += expect("root.description", "'AXDescription: New Title'"); Do we need a `waitFor` here? Maybe moving the expect into the async block is enough...but I wonder if the test is flakey or will become flakey without an explicit waitFor.
Created attachment 463762 [details] Patch
(In reply to Tyler Wilcock from comment #3) > Comment on attachment 463610 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=463610&action=review > > > LayoutTests/accessibility/mac/html-with-aria-label.html:20 > > + html.setAttribute("aria-label", "New Title"); > > I see this and some other places in this patch are using some implicit > object-by-id JS syntax. I think this not recommended: > > https://html.spec.whatwg.org/#named-access-on-the-window-object > > > As a general rule, relying on this will lead to brittle code. Which IDs end up mapping to this API can vary over time, as new features are added to the web platform, for example. Instead of this, use document.getElementById() or document.querySelector(). > > > LayoutTests/accessibility/mac/html-with-aria-label.html:22 > > + output += expect("root.description", "'AXDescription: New Title'"); > > Do we need a `waitFor` here? Maybe moving the expect into the async block is > enough...but I wonder if the test is flakey or will become flakey without an > explicit waitFor. Didn't waitFor intentionally because doesn't seem to be necessary, no failures with --iterations=1000. In real scenarios, there may not be a wait at all, so want to reduce the use of waitFor to when it is strictly necessary. Having said that, will revisit if flakiness shows up in the bots.
> Didn't waitFor intentionally because doesn't seem to be necessary, no > failures with --iterations=1000. In real scenarios, there may not be a wait > at all, so want to reduce the use of waitFor to when it is strictly > necessary. Having said that, will revisit if flakiness shows up in the bots. OK, sounds good!
Committed 257113@main (557946b122c5): <https://commits.webkit.org/257113@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 463762 [details].