Test case from Deque http://a11yideas.com/testcode/mwp/iOSsvg.html If you link an entirely inaccessible SVG (no accessible contents), WebKit should respect the @alt attribute from the linking HTML, use the outer bounds of the SVG canvas as the image bounds, and use the AXImage role instead of AXGroup.
<rdar://problem/27771579>
See related discussion from bug 145263.
Created attachment 418181 [details] Patch
Comment on attachment 418181 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418181&action=review > Source/WebCore/accessibility/AccessibilitySVGElement.cpp:78 > + for (const auto& child : childrenOfType<SVGElement>(element)) { the code block worries me. it seems like it's going to be a n! algorithm. for every child, you're going to call into this same method for every descendant. and then at the next level, you'll call that same method again
Comment on attachment 418181 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418181&action=review > Source/WebCore/accessibility/AccessibilitySVGElement.cpp:72 > + // If the role or aria-label attributes are specified, this is accessible. are there other attributes we need to check like "alt"
Comment on attachment 418181 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418181&action=review >> Source/WebCore/accessibility/AccessibilitySVGElement.cpp:78 >> + for (const auto& child : childrenOfType<SVGElement>(element)) { > > the code block worries me. it seems like it's going to be a n! algorithm. for every child, you're going to call into this same method for every descendant. and then at the next level, you'll call that same method again Using descendantsOfType instead, only at the top level, should resolve that cleanly.
Comment on attachment 418181 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418181&action=review >>> Source/WebCore/accessibility/AccessibilitySVGElement.cpp:78 >>> + for (const auto& child : childrenOfType<SVGElement>(element)) { >> >> the code block worries me. it seems like it's going to be a n! algorithm. for every child, you're going to call into this same method for every descendant. and then at the next level, you'll call that same method again > > Using descendantsOfType instead, only at the top level, should resolve that cleanly. But on reflection, I don’t think it will be n! or n^2; even in this code every descendant is visited only once.
(In reply to Darin Adler from comment #7) > Comment on attachment 418181 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=418181&action=review > > >>> Source/WebCore/accessibility/AccessibilitySVGElement.cpp:78 > >>> + for (const auto& child : childrenOfType<SVGElement>(element)) { > >> > >> the code block worries me. it seems like it's going to be a n! algorithm. for every child, you're going to call into this same method for every descendant. and then at the next level, you'll call that same method again > > > > Using descendantsOfType instead, only at the top level, should resolve that cleanly. > > But on reflection, I don’t think it will be n! or n^2; even in this code > every descendant is visited only once. yea I was thinking if this has to be called at every level of the tree for every element, we'd end up descending a lot
Comment on attachment 418181 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418181&action=review >>>>> Source/WebCore/accessibility/AccessibilitySVGElement.cpp:78 >>>>> + for (const auto& child : childrenOfType<SVGElement>(element)) { >>>> >>>> the code block worries me. it seems like it's going to be a n! algorithm. for every child, you're going to call into this same method for every descendant. and then at the next level, you'll call that same method again >>> >>> Using descendantsOfType instead, only at the top level, should resolve that cleanly. >> >> But on reflection, I don’t think it will be n! or n^2; even in this code every descendant is visited only once. > > yea I was thinking if this has to be called at every level of the tree for every element, we'd end up descending a lot Oh, yes this is O(descendants) and it’s *not* OK to call something like that for all elements. Could be a real challenge to deal with that.
Comment on attachment 418181 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418181&action=review >>>>>> Source/WebCore/accessibility/AccessibilitySVGElement.cpp:78 >>>>>> + for (const auto& child : childrenOfType<SVGElement>(element)) { >>>>> >>>>> the code block worries me. it seems like it's going to be a n! algorithm. for every child, you're going to call into this same method for every descendant. and then at the next level, you'll call that same method again >>>> >>>> Using descendantsOfType instead, only at the top level, should resolve that cleanly. >>> >>> But on reflection, I don’t think it will be n! or n^2; even in this code every descendant is visited only once. >> >> yea I was thinking if this has to be called at every level of the tree for every element, we'd end up descending a lot > > Oh, yes this is O(descendants) and it’s *not* OK to call something like that for all elements. > > Could be a real challenge to deal with that. talking to Andres offline we determined this would only be called once and only on the root. he had some ideas on how to make that more clear in the code
Created attachment 418229 [details] Patch
(In reply to chris fleizach from comment #10) > Comment on attachment 418181 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=418181&action=review > > >>>>>> Source/WebCore/accessibility/AccessibilitySVGElement.cpp:78 > >>>>>> + for (const auto& child : childrenOfType<SVGElement>(element)) { > >>>>> > >>>>> the code block worries me. it seems like it's going to be a n! algorithm. for every child, you're going to call into this same method for every descendant. and then at the next level, you'll call that same method again > >>>> > >>>> Using descendantsOfType instead, only at the top level, should resolve that cleanly. > >>> > >>> But on reflection, I don’t think it will be n! or n^2; even in this code every descendant is visited only once. > >> > >> yea I was thinking if this has to be called at every level of the tree for every element, we'd end up descending a lot > > > > Oh, yes this is O(descendants) and it’s *not* OK to call something like that for all elements. > > > > Could be a real challenge to deal with that. > > talking to Andres offline we determined this would only be called once and > only on the root. he had some ideas on how to make that more clear in the > code I moved the hasAccessibleContent method to the AccessibilitySVGRoot class which makes it clearer that this method would be called only on the SVG root. Also used the descendantsOfType function suggested by Darin that makes it a lot cleaner. Thank you both very much for the feedback.
(In reply to chris fleizach from comment #5) > Comment on attachment 418181 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=418181&action=review > > > Source/WebCore/accessibility/AccessibilitySVGElement.cpp:72 > > + // If the role or aria-label attributes are specified, this is accessible. > > are there other attributes we need to check like "alt" Investigating this. have to consult the ARIA and SVG accessibility specs.
One new test failure https://ews-build.s3-us-west-2.amazonaws.com/macOS-Mojave-Release-WK1-Tests-EWS/r418229-26105/results.html
Created attachment 418295 [details] Patch
Committed r271796: <https://trac.webkit.org/changeset/271796> All reviewed patches have been landed. Closing bug and clearing flags on attachment 418295 [details].
*** Bug 202431 has been marked as a duplicate of this bug. ***