AX: role="none" (or presentation) does not work on iframes

[James Craig]

Created attachment 314041 [details]
test case

AX: role="none" (or presentation) does not work on iframes

Load attached test case. The frame should not be exposed as an AXWebArea. VoiceOver and other AT should only see the contents, not the document borders. This should work more or less like @seamless used to work before it was removed.

[Radar WebKit Bug Importer]

<rdar://problem/33034347>

[chris fleizach]

Created attachment 314374 [details]
patch

[Build Bot]

Attachment 314374 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1198:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/accessibility/AccessibilityObject.h:484:  The parameter name "node" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[chris fleizach]

Created attachment 314376 [details]
patch

[Build Bot]

Attachment 314376 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1198:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 314376 [details]
patch

Attachment 314376 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4032951

New failing tests:
accessibility/aria-hidden-false-works-in-subtrees.html
accessibility/mac/webkit-scrollarea-position.html

[Build Bot]

Created attachment 314379 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[chris fleizach]

Created attachment 314393 [details]
patch

[Build Bot]

Attachment 314393 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1198:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 314393 [details]
patch

Attachment 314393 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4036577

New failing tests:
workers/bomb.html

[Build Bot]

Created attachment 314396 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Ryosuke Niwa]

Comment on attachment 314393 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=314393&action=review

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1161
> +    if (!is<RenderView>(*renderer))
> +        return false;
> +    
> +    if (!renderer->document().ownerElement())
> +        return false;

Rather than checking if render object being a render view,
why don't we just check that the node is not HTMLFrameOwnerElement?
That way, it'd work with object element loading a HTML document as well.

[chris fleizach]

(In reply to Ryosuke Niwa from comment #12)
> Comment on attachment 314393 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=314393&action=review
> 
> > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1161
> > +    if (!is<RenderView>(*renderer))
> > +        return false;
> > +    
> > +    if (!renderer->document().ownerElement())
> > +        return false;
> 
> Rather than checking if render object being a render view,
> why don't we just check that the node is not HTMLFrameOwnerElement?
> That way, it'd work with object element loading a HTML document as well.

Might be missing something here. For this method, I want it to tell me that it is a AXWebArea (which is essentially a RenderView) and that the frame owner element's role is presentational. If I remove the RenderView check I think it might sweep up other elements too.

[Ryosuke Niwa]

(In reply to chris fleizach from comment #13)
> (In reply to Ryosuke Niwa from comment #12)
> > Comment on attachment 314393 [details]
> > patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=314393&action=review
> > 
> > > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1161
> > > +    if (!is<RenderView>(*renderer))
> > > +        return false;
> > > +    
> > > +    if (!renderer->document().ownerElement())
> > > +        return false;
> > 
> > Rather than checking if render object being a render view,
> > why don't we just check that the node is not HTMLFrameOwnerElement?
> > That way, it'd work with object element loading a HTML document as well.
> 
> Might be missing something here. For this method, I want it to tell me that
> it is a AXWebArea (which is essentially a RenderView) and that the frame
> owner element's role is presentational. If I remove the RenderView check I
> think it might sweep up other elements too.

Oh I see so RenderView check is needed. Nut even in that case, don't we still want to support this for object element with role=none? There's virtually no difference to object and iframe elements in this regard.

[chris fleizach]

(In reply to Ryosuke Niwa from comment #14)
> (In reply to chris fleizach from comment #13)
> > (In reply to Ryosuke Niwa from comment #12)
> > > Comment on attachment 314393 [details]
> > > patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=314393&action=review
> > > 
> > > > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1161
> > > > +    if (!is<RenderView>(*renderer))
> > > > +        return false;
> > > > +    
> > > > +    if (!renderer->document().ownerElement())
> > > > +        return false;
> > > 
> > > Rather than checking if render object being a render view,
> > > why don't we just check that the node is not HTMLFrameOwnerElement?
> > > That way, it'd work with object element loading a HTML document as well.
> > 
> > Might be missing something here. For this method, I want it to tell me that
> > it is a AXWebArea (which is essentially a RenderView) and that the frame
> > owner element's role is presentational. If I remove the RenderView check I
> > think it might sweep up other elements too.
> 
> Oh I see so RenderView check is needed. Nut even in that case, don't we
> still want to support this for object element with role=none? There's
> virtually no difference to object and iframe elements in this regard.

Didn't realize you could do that with object. It looks like <object> will create a AXWebArea so I'll look into this to see what else we need to do to support

[chris fleizach]

Created attachment 314419 [details]
patch

Updated to make work with <object>

[Build Bot]

Attachment 314419 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1195:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ryosuke Niwa]

Comment on attachment 314419 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=314419&action=review

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1196
> +    // WebAreas should be ignored if their iframe container is marked as presentational.
> +    if (webAreaIsPresentational(m_renderer))
> +        return true;

Somehow the style bot is complaining about this code but there's nothing wrong with this code.

[Build Bot]

Comment on attachment 314419 [details]
patch

Attachment 314419 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4039026

New failing tests:
accessibility/presentation-role-iframe.html

[Build Bot]

Created attachment 314420 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[chris fleizach]

Created attachment 314421 [details]
patch

Minor updates to test

[Build Bot]

Comment on attachment 314419 [details]
patch

Attachment 314419 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4039091

New failing tests:
accessibility/presentation-role-iframe.html

[Build Bot]

Created attachment 314422 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Attachment 314421 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1195:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 314419 [details]
patch

Attachment 314419 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4039129

New failing tests:
accessibility/presentation-role-iframe.html

[Build Bot]

Created attachment 314425 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[WebKit Commit Bot]

Comment on attachment 314421 [details]
patch

Rejecting attachment 314421 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 314421, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/4044916

[chris fleizach]

Created attachment 314488 [details]
real patch for landing

[Build Bot]

Attachment 314488 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1195:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 314488 [details]
real patch for landing

Clearing flags on attachment: 314488

Committed r219075: <http://trac.webkit.org/changeset/219075>

[Ahmad Saleem]

Landed and didn't backed out:

https://github.com/WebKit/WebKit/commit/311f7954d3a0a280bb6531383c7194efb897c08d

Marking this as "RESOLVED FIXED".