Bug 263715

Summary: AX: Add test coverage for setting focus inside a modal dialog.
Product: WebKit Reporter: Andres Gonzalez <andresg_22>
Component: AccessibilityAssignee: Andres Gonzalez <andresg_22>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, apinheiro, cfleizach, cgarcia, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, tyler_w, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
andresg_22: commit-queue+
Patch none

Andres Gonzalez
Reported 2023-10-26 05:47:37 PDT
.
Attachments
Patch (3.84 KB, patch)
2023-10-26 06:17 PDT, Andres Gonzalez 		no flags
DetailsFormatted DiffDiff
Patch (4.97 KB, patch)
2023-10-26 14:32 PDT, Andres Gonzalez 		no flags
DetailsFormatted DiffDiff
Patch (5.75 KB, patch)
2023-11-03 07:21 PDT, Andres Gonzalez 		andresg_22: commit-queue+
DetailsFormatted DiffDiff
Patch (5.75 KB, patch)
2023-11-03 11:24 PDT, Andres Gonzalez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2023-10-26 05:47:47 PDT
<rdar://problem/117530301>
Andres Gonzalez
Comment 2 2023-10-26 06:17:52 PDT
Created attachment 468351 [details] Patch
Tyler Wilcock
Comment 3 2023-10-26 10:24:36 PDT
Comment on attachment 468351 [details] Patch Looks good, but failing GTK + WPE bots
Andres Gonzalez
Comment 4 2023-10-26 14:32:16 PDT
Created attachment 468361 [details] Patch Follow up fix for ATSPI.
Carlos Garcia Campos
Comment 5 2023-10-30 07:07:13 PDT
I'll check the atspi failure as soon as I find some time.
Andres Gonzalez
Comment 6 2023-11-03 07:21:38 PDT
Created attachment 468469 [details] Patch
Tyler Wilcock
Comment 7 2023-11-03 09:27:30 PDT
Comment on attachment 468469 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=468469&action=review > LayoutTests/platform/glib/TestExpectations:460 > +accessibility/focus-inside-modal.html Do we have have to specify [ Skip ] or [ Timeout ]?
Andres Gonzalez
Comment 8 2023-11-03 11:24:17 PDT
Created attachment 468474 [details] Patch
Andres Gonzalez
Comment 9 2023-11-03 11:26:54 PDT
(In reply to Tyler Wilcock from comment #7) > Comment on attachment 468469 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=468469&action=review > > > LayoutTests/platform/glib/TestExpectations:460 > > +accessibility/focus-inside-modal.html > > Do we have have to specify [ Skip ] or [ Timeout ]? Yes! hadn't had enough coffee this morning, thanks!
Carlos Garcia Campos
Comment 10 2023-11-03 23:20:51 PDT
Comment on attachment 468474 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=468474&action=review > COMMIT_MESSAGE:15 > +In addition the following ATSPI fix is needed for the test to pass in this platform (follow up to https://bugs.webkit.org/show_bug.cgi?id=263678): > +* Tools/WebKitTestRunner/InjectedBundle/atspi/AccessibilityNotificationHandler.cpp: > +(WTR::AccessibilityNotificationHandler::AccessibilityNotificationHandler): I'm confused, this change is needed to make it pass, but you are also skipping it saying it times out.
Carlos Garcia Campos
Comment 11 2023-11-03 23:22:42 PDT
I'm sorry I haven't been able to look at this yet, it's been a busy week. I'm fine to land this with the test marked as failure to not block you.
Tyler Wilcock
Comment 12 2023-11-06 09:25:36 PST
(In reply to Carlos Garcia Campos from comment #10) > Comment on attachment 468474 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=468474&action=review > > > COMMIT_MESSAGE:15 > > +In addition the following ATSPI fix is needed for the test to pass in this platform (follow up to https://bugs.webkit.org/show_bug.cgi?id=263678): > > +* Tools/WebKitTestRunner/InjectedBundle/atspi/AccessibilityNotificationHandler.cpp: > > +(WTR::AccessibilityNotificationHandler::AccessibilityNotificationHandler): > > I'm confused, this change is needed to make it pass, but you are also > skipping it saying it times out. Andres, do you know if the test is passing after making this change, meaning we don't need to mark it as skip / timeout?
EWS
Comment 13 2023-11-06 09:34:30 PST
Committed 270272@main (3de34e00862a): <https://commits.webkit.org/270272@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 468474 [details].
Andres Gonzalez
Comment 14 2023-11-06 11:01:01 PST
(In reply to Carlos Garcia Campos from comment #10) > Comment on attachment 468474 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=468474&action=review > > > COMMIT_MESSAGE:15 > > +In addition the following ATSPI fix is needed for the test to pass in this platform (follow up to https://bugs.webkit.org/show_bug.cgi?id=263678): > > +* Tools/WebKitTestRunner/InjectedBundle/atspi/AccessibilityNotificationHandler.cpp: > > +(WTR::AccessibilityNotificationHandler::AccessibilityNotificationHandler): > > I'm confused, this change is needed to make it pass, but you are also > skipping it saying it times out. I believe that change is necessary as it is in COCOA, but it wasn't sufficient for ATSPI, still failing after that change. That's why I marked it as skip.
Note You need to log in before you can comment on or make changes to this bug.