Bug 173930 - AX: role="none" (or presentation) does not work on iframes
Summary: AX: role="none" (or presentation) does not work on iframes
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-06-28 10:56 PDT by James Craig
Modified: 2018-02-23 08:55 PST (History)
12 users (show)

See Also:


Attachments
test case (231 bytes, text/html)
2017-06-28 10:56 PDT, James Craig
no flags Details
patch (15.56 KB, patch)
2017-07-01 00:32 PDT, chris fleizach
no flags Details | Formatted Diff | Diff
patch (15.55 KB, patch)
2017-07-01 01:44 PDT, chris fleizach
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (1.38 MB, application/zip)
2017-07-01 02:48 PDT, Build Bot
no flags Details
patch (15.56 KB, patch)
2017-07-01 17:58 PDT, chris fleizach
rniwa: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-elcapitan (1.15 MB, application/zip)
2017-07-01 19:34 PDT, Build Bot
no flags Details
patch (15.82 KB, patch)
2017-07-02 12:14 PDT, chris fleizach
rniwa: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (1.38 MB, application/zip)
2017-07-02 13:02 PDT, Build Bot
no flags Details
patch (15.85 KB, patch)
2017-07-02 13:12 PDT, chris fleizach
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-elcapitan (1018.49 KB, application/zip)
2017-07-02 13:14 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews112 for mac-elcapitan (1.78 MB, application/zip)
2017-07-02 13:42 PDT, Build Bot
no flags Details
real patch for landing (15.79 KB, patch)
2017-07-03 09:19 PDT, chris fleizach
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Craig 2017-06-28 10:56:53 PDT
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.
Comment 1 Radar WebKit Bug Importer 2017-06-28 10:57:29 PDT
<rdar://problem/33034347>
Comment 2 chris fleizach 2017-07-01 00:32:45 PDT
Created attachment 314374 [details]
patch
Comment 3 Build Bot 2017-07-01 00:34:26 PDT
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.
Comment 4 chris fleizach 2017-07-01 01:44:45 PDT
Created attachment 314376 [details]
patch
Comment 5 Build Bot 2017-07-01 01:47:55 PDT
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.
Comment 6 Build Bot 2017-07-01 02:48:43 PDT
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
Comment 7 Build Bot 2017-07-01 02:48:44 PDT
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
Comment 8 chris fleizach 2017-07-01 17:58:34 PDT
Created attachment 314393 [details]
patch
Comment 9 Build Bot 2017-07-01 18:01:03 PDT
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.
Comment 10 Build Bot 2017-07-01 19:34:55 PDT
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
Comment 11 Build Bot 2017-07-01 19:34:56 PDT
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
Comment 12 Ryosuke Niwa 2017-07-01 20:25:01 PDT
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.
Comment 13 chris fleizach 2017-07-02 00:20:42 PDT
(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.
Comment 14 Ryosuke Niwa 2017-07-02 00:58:00 PDT
(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.
Comment 15 chris fleizach 2017-07-02 11:54:05 PDT
(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
Comment 16 chris fleizach 2017-07-02 12:14:29 PDT
Created attachment 314419 [details]
patch

Updated to make work with <object>
Comment 17 Build Bot 2017-07-02 12:17:35 PDT
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.
Comment 18 Ryosuke Niwa 2017-07-02 12:27:26 PDT
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.
Comment 19 Build Bot 2017-07-02 13:02:33 PDT
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
Comment 20 Build Bot 2017-07-02 13:02:34 PDT
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
Comment 21 chris fleizach 2017-07-02 13:12:33 PDT
Created attachment 314421 [details]
patch

Minor updates to test
Comment 22 Build Bot 2017-07-02 13:14:45 PDT
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
Comment 23 Build Bot 2017-07-02 13:14:46 PDT
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
Comment 24 Build Bot 2017-07-02 13:15:15 PDT
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.
Comment 25 Build Bot 2017-07-02 13:42:23 PDT
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
Comment 26 Build Bot 2017-07-02 13:42:25 PDT
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
Comment 27 WebKit Commit Bot 2017-07-03 08:53:25 PDT
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
Comment 28 chris fleizach 2017-07-03 09:19:48 PDT
Created attachment 314488 [details]
real patch for landing
Comment 29 Build Bot 2017-07-03 09:22:18 PDT
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.
Comment 30 WebKit Commit Bot 2017-07-03 10:18:38 PDT
Comment on attachment 314488 [details]
real patch for landing

Clearing flags on attachment: 314488

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