WebKit doesn't call [super -accessibilityAttributeValue:attribute forParameter:] when it encounters a parameterized attribute that it doesn't handle.
Created attachment 56417 [details] patch
Comment on attachment 56417 [details] patch > - // got a parameter of a type we never use > - // NOTE: No ASSERT_NOT_REACHED because this can happen accidentally > - // while using accesstool (e.g.), forcing you to start over > - return nil; > + // Default to super if the parameter type is unknown. > + return [super accessibilityAttributeValue:attribute forParameter:parameter]; Is this right? It's a parameter we support, but a type we don't. I don't think calling super makes a lot of sense in this case, but I guess it's harmless. > - return nil; > + // Default to super if parameter is not supported. > + return [super accessibilityAttributeValue:attribute forParameter:parameter]; This seems clearly right.
(In reply to comment #2) > (From update of attachment 56417 [details]) > > - // got a parameter of a type we never use > > - // NOTE: No ASSERT_NOT_REACHED because this can happen accidentally > > - // while using accesstool (e.g.), forcing you to start over > > - return nil; > > + // Default to super if the parameter type is unknown. > > + return [super accessibilityAttributeValue:attribute forParameter:parameter]; > > Is this right? It's a parameter we support, but a type we don't. I don't think calling super makes a lot of sense in this case, but I guess it's harmless. At this point, it's still not clear if its a parameter we support. its only clear that its an unsupported type (but it might be an type supported by super)
Comment on attachment 56417 [details] patch Rejecting patch 56417 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--ignore-tests', 'compositing/iframes', '--quiet']" exit_code: 1 Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Skipped list contained 'compositing/iframes/composited-iframe.html', but no file of that name could be found Testing 18369 test cases. accessibility/aria-tables.html -> crashed Exiting early after 1 failures. 20 tests run. 171.06s total testing time 19 test cases (95%) succeeded 1 test case (5%) crashed Full output: http://webkit-commit-queue.appspot.com/results/2276405
If this causes failure on leopard, I'll have to see whats the backtrace. http://trac.webkit.org/changeset/60307
it looks like on tiger this idiom doesn't work as documented
and on leopard it looks like it crashes. whereas on tiger it times out a few tests
working on fix for tiger and leopard...
conferring with appkit that this should not work in leopard and before, but will work going forward
http://trac.webkit.org/changeset/60307 might have broken Tiger Intel Release The following changes are on the blame list: http://trac.webkit.org/changeset/60305 http://trac.webkit.org/changeset/60306 http://trac.webkit.org/changeset/60307 http://trac.webkit.org/changeset/60308
rolling out http://trac.webkit.org/changeset/60310 until i have a better fix (which required more code than i thought, so i will open for review again)
here's the deal we want to pass unknown attributes to super. However, if an unsupported attribute is passed in it raises an accessibility exception. Normally those are caught by the AX runtime so that it can inform the AX client (like VoiceOver). However, for these failing tests, DRT is the AX client, but its not catching those innocuous errors. DRT needs to catch those errors
Created attachment 57279 [details] Patch
Comment on attachment 57279 [details] Patch i'll break this up into smaller patches
with https://bugs.webkit.org/show_bug.cgi?id=39880 and https://bugs.webkit.org/show_bug.cgi?id=39881 fixed, this patch should work
Created attachment 57347 [details] Patch
Comment on attachment 57347 [details] Patch I am going to r+ this, but do you think there is any particular reason why this should be a goto instead of a regular function call? I shy away from using gotos since there aren't a lot of them in the code, but since we don't have an official policy, r+ anyway. I would like it if you would answer my question though ;-)
(In reply to comment #17) > (From update of attachment 57347 [details]) > I am going to r+ this, but do you think there is any particular reason why this should be a goto instead of a regular function call? I shy away from using gotos since there aren't a lot of them in the code, but since we don't have an official policy, r+ anyway. I would like it if you would answer my question though ;-) using the goto allowed be to consolidate the comment. i saw a few other uses of goto, so i figured it was acceptable. i'm generally against them as well, but it looked ok here. what do you think about the comment... is it ok just having it at the bottom, despite [super] being called in two places?
I think the comment is fine in one place :-)
(In reply to comment #19) > I think the comment is fine in one place :-) thanx. will make the change
http://trac.webkit.org/changeset/60496