Bug 120117 - AX: <noscript> contents are exposed as static text
Summary: AX: <noscript> contents are exposed as static text
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-08-21 09:25 PDT by chris fleizach
Modified: 2013-08-27 14:56 PDT (History)
10 users (show)

See Also:


Attachments
patch (3.72 KB, patch)
2013-08-21 13:33 PDT, chris fleizach
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (570.91 KB, application/zip)
2013-08-21 16:27 PDT, Build Bot
no flags Details
patch (4.60 KB, patch)
2013-08-22 09:15 PDT, chris fleizach
no flags Details | Formatted Diff | Diff
patch (4.59 KB, patch)
2013-08-22 09:16 PDT, chris fleizach
thorton: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2013-08-21 09:25:15 PDT
If a <canvas> object using <noscript>, then the text contents of no script are exposed as AX elements
Comment 1 Radar WebKit Bug Importer 2013-08-21 09:25:26 PDT
<rdar://problem/14796451>
Comment 2 chris fleizach 2013-08-21 09:25:36 PDT
<rdar://problem/14775259>
Comment 3 chris fleizach 2013-08-21 13:33:17 PDT
Created attachment 209299 [details]
patch
Comment 4 Build Bot 2013-08-21 16:27:36 PDT
Comment on attachment 209299 [details]
patch

Attachment 209299 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1516886

New failing tests:
accessibility/noscript-ignored.html
Comment 5 Build Bot 2013-08-21 16:27:37 PDT
Created attachment 209313 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.4
Comment 6 chris fleizach 2013-08-21 16:41:48 PDT
this test is weirdly flaky for some reason. Maybe the bit field isn't big enough to hold the default value?
Comment 7 chris fleizach 2013-08-22 09:15:13 PDT
Created attachment 209372 [details]
patch

Actually just forgot a file.
Comment 8 chris fleizach 2013-08-22 09:16:54 PDT
Created attachment 209373 [details]
patch
Comment 9 Build Bot 2013-08-26 20:08:04 PDT
Comment on attachment 209373 [details]
patch

Attachment 209373 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/1586122
Comment 10 chris fleizach 2013-08-27 09:01:06 PDT
This Windows build failure seems unrelated. The TestWebKitAPI target seems to be the one that is failing
Comment 11 Tim Horton 2013-08-27 11:05:16 PDT
Comment on attachment 209373 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2873
> +    // Add the unrendered canvas children as AX nodes, unless we're not using a canvas renderer
> +    // because JS is disabled for example.
> +    if (!node() || !node()->hasTagName(canvasTag) || (renderer() && !renderer()->isCanvas()))

Will we still get the <noscript> if JS is disabled in this case?

> LayoutTests/accessibility/noscript-ignored.html:8
> +<canvas width="200" height="100" id="canvas"><noscript><img id='hidden-image' src='resources/cake.png'></noscript></canvas>

Can we test the JS-disabled case too?

Does noscript work with anything other than canvas? (fairly sure the answer is yes). You've special cased canvas; do you need to do that for anything else?
Comment 12 chris fleizach 2013-08-27 12:16:18 PDT
(In reply to comment #11)
> (From update of attachment 209373 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=209373&action=review
> 
> > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2873
> > +    // Add the unrendered canvas children as AX nodes, unless we're not using a canvas renderer
> > +    // because JS is disabled for example.
> > +    if (!node() || !node()->hasTagName(canvasTag) || (renderer() && !renderer()->isCanvas()))
> 
> Will we still get the <noscript> if JS is disabled in this case?
> 

If JS is disabled, then the children are rendered and this code path is not used

> > LayoutTests/accessibility/noscript-ignored.html:8
> > +<canvas width="200" height="100" id="canvas"><noscript><img id='hidden-image' src='resources/cake.png'></noscript></canvas>
> 
> Can we test the JS-disabled case too?

> 
> Does noscript work with anything other than canvas? (fairly sure the answer is yes). You've special cased canvas; do you need to do that for anything else?

Right now, canvas is the only element that we're adding non-rendered children for (so we can implement the accessible shadow DOM for cavnii). So this problem won't occur for other examples yet (like WebGL maybe)
Comment 13 chris fleizach 2013-08-27 12:16:49 PDT
(In reply to comment #11)
> (From update of attachment 209373 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=209373&action=review
> 
> > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2873
> > +    // Add the unrendered canvas children as AX nodes, unless we're not using a canvas renderer
> > +    // because JS is disabled for example.
> > +    if (!node() || !node()->hasTagName(canvasTag) || (renderer() && !renderer()->isCanvas()))
> 
> Will we still get the <noscript> if JS is disabled in this case?
> 
> > LayoutTests/accessibility/noscript-ignored.html:8
> > +<canvas width="200" height="100" id="canvas"><noscript><img id='hidden-image' src='resources/cake.png'></noscript></canvas>
> 
> Can we test the JS-disabled case too?

Does anyone know how to make a JS disable layout test (we're you'd still need to use JS to access the AX tree?)

> 
> Does noscript work with anything other than canvas? (fairly sure the answer is yes). You've special cased canvas; do you need to do that for anything else?
Comment 14 Tim Horton 2013-08-27 12:55:29 PDT
(In reply to comment #13)
> (In reply to comment #11)
> > > LayoutTests/accessibility/noscript-ignored.html:8
> > > +<canvas width="200" height="100" id="canvas"><noscript><img id='hidden-image' src='resources/cake.png'></noscript></canvas>
> > 
> > Can we test the JS-disabled case too?
> 
> Does anyone know how to make a JS disable layout test (we're you'd still need to use JS to access the AX tree?)

Heh, probably not.
Comment 15 chris fleizach 2013-08-27 14:56:42 PDT
http://trac.webkit.org/changeset/154710