Created attachment 178416 [details] A layout test that should pass but it isn't If a ListBox has a disabled item it should be exposed as disabled. I found this problem while working on the bug 98376 I found a new problem. The bug is about getting the gtk port passing a existing test. On that test the last item is not selectable. But in the same way, it shouldn't be enabled, but it is. I'm attaching a modified test, with the expected outcome. Right now this is failing. Note: I'm reporting this bug to the GTK port, as it is the one I'm working on, but this test should also pass in any port, and I think that this would be failing on any other port.
Created attachment 178417 [details] Fixes the bug The error was at AccessibilityListBoxOption, as it redefined isEnabled, and didn't take into account the DISABLED attribute. So this patch takes that into account, and for completeness sake, I also added a check for aria-disabled. I renamed the test, as this is a listbox specif thing, and also added aria-atribute checks. As you could have noticed, I also removed the [GTK] prefix at the summary and moved it to the Accessibility component. This is because taking into account the solution, this problem would be affecting any port.
Chris, could you please review this? As I said is a really easy patch.
Comment on attachment 178417 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=178417&action=review It bugs me a bit that we're checking for aria_disabledAttr in two methods, but I see a bunch of different implementations that would make a common implementation a bit tricky and less glamorous to get right. I think if we continue to provide custom isEnabled implementations we should think about refactoring a bit, but I think this is OK in current form just a minor comment about the layout tests > LayoutTests/accessibility/listbox-enabled-states.html:35 > + shouldBeTrue('accessibilityController.focusedElement.childAtIndex(4).isEnabled'); you should also include the js-post test scripts so that we get the successfullyParsed messages > Source/WebCore/ChangeLog:3 > + Wrong Enabled states on a ListBox We should probably prefix the bug title with AX: * so that others know what this refers to
Created attachment 178504 [details] Fixes the bug (In reply to comment #3) > (From update of attachment 178417 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=178417&action=review > > It bugs me a bit that we're checking for aria_disabledAttr in two methods, but I see a bunch of different implementations that would make a common implementation a bit tricky and less glamorous to get right. > I think if we continue to provide custom isEnabled implementations we should think about refactoring a bit, but I think this is OK in current form I agree. > just a minor comment about the layout tests > > > LayoutTests/accessibility/listbox-enabled-states.html:35 > > + shouldBeTrue('accessibilityController.focusedElement.childAtIndex(4).isEnabled'); > > you should also include the js-post test scripts so that we get the successfullyParsed messages Done. Note: I based this tests on a existing one, so the reasons I didn't notice this. Right now there are several tests that lack the js-post. Anyway not sure if a "cleaning task" it is worth or not. > > > Source/WebCore/ChangeLog:3 > > + Wrong Enabled states on a ListBox > > We should probably prefix the bug title with AX: * > so that others know what this refers to Done
Comment on attachment 178504 [details] Fixes the bug Clearing flags on attachment: 178504 Committed r137168: <http://trac.webkit.org/changeset/137168>
All reviewed patches have been landed. Closing bug.