WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
104483
AX: wrong Enabled states on a ListBox
https://bugs.webkit.org/show_bug.cgi?id=104483
Summary
AX: wrong Enabled states on a ListBox
Alejandro Piñeiro
Reported
2012-12-09 05:55:38 PST
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.
Attachments
A layout test that should pass but it isn't
(2.46 KB, patch)
2012-12-09 05:55 PST
,
Alejandro Piñeiro
no flags
Details
Formatted Diff
Diff
Fixes the bug
(5.19 KB, patch)
2012-12-09 07:36 PST
,
Alejandro Piñeiro
no flags
Details
Formatted Diff
Diff
Fixes the bug
(5.31 KB, patch)
2012-12-10 03:08 PST
,
Alejandro Piñeiro
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Alejandro Piñeiro
Comment 1
2012-12-09 07:36:52 PST
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.
Alejandro Piñeiro
Comment 2
2012-12-09 08:39:14 PST
Chris, could you please review this? As I said is a really easy patch.
chris fleizach
Comment 3
2012-12-09 23:35:57 PST
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
Alejandro Piñeiro
Comment 4
2012-12-10 03:08:52 PST
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
WebKit Review Bot
Comment 5
2012-12-10 09:31:23 PST
Comment on
attachment 178504
[details]
Fixes the bug Clearing flags on attachment: 178504 Committed
r137168
: <
http://trac.webkit.org/changeset/137168
>
WebKit Review Bot
Comment 6
2012-12-10 09:31:26 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug