Bug 137669

Summary: AX: [ATK] CSS-generated text content not exposed to assistive technologies
Product: WebKit Reporter: Joanmarie Diggs <jdiggs>
Component: AccessibilityAssignee: Joanmarie Diggs <jdiggs>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, jcraig, mario, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: Linux   
Bug Depends on:    
Bug Blocks: 25531    
Attachments:
Description Flags
Patch none

Description Joanmarie Diggs 2014-10-13 13:53:18 PDT
Given:

  <html>
  <head>
  <style type="text/css">
  p#generated:before { content: "Before"; }
  p#generated:after { content: "After"; }
  </style>
  </head>
  <body>
  <p id="generated"> Text </p>
  </body>
  </html>

The reported accessible text for the paragraph is " Text ".
Comment 1 Radar WebKit Bug Importer 2014-10-13 13:53:44 PDT
<rdar://problem/18638498>
Comment 2 Joanmarie Diggs 2014-10-13 14:12:26 PDT
Created attachment 239742 [details]
Patch
Comment 3 Joanmarie Diggs 2014-10-13 14:19:12 PDT
See also bug 137672.
Comment 4 chris fleizach 2014-10-13 14:19:35 PDT
Note: this seems to work ok for me with VoiceOver on a mac
Comment 5 chris fleizach 2014-10-13 14:21:07 PDT
Comment on attachment 239742 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        Test: accessibility/css-content-attribute.html

how come this works on Mac, but not GTK without more modification?
Comment 6 Joanmarie Diggs 2014-10-13 16:55:05 PDT
(In reply to comment #5)
> (From update of attachment 239742 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=239742&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        Test: accessibility/css-content-attribute.html
> 
> how come this works on Mac, but not GTK without more modification?

Good question. Here's what seems to be the case (please correct me if I'm wrong):

OS X most of the time is not calling textUnderElement on RenderBlock elements. Instead, you ask for and get the text for the children of those elements (i.e. the RenderInline and RenderText objects). The exception to this rule seems to be when you're getting the title for objects such as headings. But in that case, the mode is TextUnderElementModeSkipIgnoredChildren so you wind up deferring to AccessibilityNodeObject::textUnderElement.

In contrast, GTK most of the time IS calling textUnderElement on RenderBlock elements and when it does the mode is TextUnderElementModeIncludeAllChildren because for GTK/ATK, StaticTextRole objects are "ignored" -- their content being folded into the parent block element. But as a result of this mode difference, on GTK, we're not deferring to AccessibilityNodeObject::textUnderElement.

In light of the above, I wonder if we could simplify AccessibilityRenderObject::textUnderElement. How much benefit are we getting from our attempts to use a text iterator for consistent handling of whitespace?
Comment 7 chris fleizach 2014-10-21 09:19:45 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 239742 [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=239742&action=review
> > 
> > > Source/WebCore/ChangeLog:11
> > > +        Test: accessibility/css-content-attribute.html
> > 
> > how come this works on Mac, but not GTK without more modification?
> 
> Good question. Here's what seems to be the case (please correct me if I'm
> wrong):
> 
> OS X most of the time is not calling textUnderElement on RenderBlock
> elements. Instead, you ask for and get the text for the children of those
> elements (i.e. the RenderInline and RenderText objects). The exception to
> this rule seems to be when you're getting the title for objects such as
> headings. But in that case, the mode is
> TextUnderElementModeSkipIgnoredChildren so you wind up deferring to
> AccessibilityNodeObject::textUnderElement.
> 
> In contrast, GTK most of the time IS calling textUnderElement on RenderBlock
> elements and when it does the mode is TextUnderElementModeIncludeAllChildren
> because for GTK/ATK, StaticTextRole objects are "ignored" -- their content
> being folded into the parent block element. But as a result of this mode
> difference, on GTK, we're not deferring to
> AccessibilityNodeObject::textUnderElement.
> 
> In light of the above, I wonder if we could simplify
> AccessibilityRenderObject::textUnderElement. How much benefit are we getting
> from our attempts to use a text iterator for consistent handling of
> whitespace?

(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 239742 [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=239742&action=review
> > 
> > > Source/WebCore/ChangeLog:11
> > > +        Test: accessibility/css-content-attribute.html
> > 
> > how come this works on Mac, but not GTK without more modification?
> 
> Good question. Here's what seems to be the case (please correct me if I'm
> wrong):
> 
> OS X most of the time is not calling textUnderElement on RenderBlock
> elements. Instead, you ask for and get the text for the children of those
> elements (i.e. the RenderInline and RenderText objects). The exception to
> this rule seems to be when you're getting the title for objects such as
> headings. But in that case, the mode is
> TextUnderElementModeSkipIgnoredChildren so you wind up deferring to
> AccessibilityNodeObject::textUnderElement.
> 
> In contrast, GTK most of the time IS calling textUnderElement on RenderBlock
> elements and when it does the mode is TextUnderElementModeIncludeAllChildren
> because for GTK/ATK, StaticTextRole objects are "ignored" -- their content
> being folded into the parent block element. But as a result of this mode
> difference, on GTK, we're not deferring to
> AccessibilityNodeObject::textUnderElement.
> 
> In light of the above, I wonder if we could simplify
> AccessibilityRenderObject::textUnderElement. How much benefit are we getting
> from our attempts to use a text iterator for consistent handling of
> whitespace?

Simplifying these these methods seems like it would be a good idea. It's totally unclear at this time what is expected behavior without actually debugging the code.

Do you want to do that with this patch, or do you want to get this change in?
Comment 8 Joanmarie Diggs 2014-10-21 10:15:27 PDT
(In reply to comment #7)

> Simplifying these these methods seems like it would be a good idea. It's
> totally unclear at this time what is expected behavior without actually
> debugging the code.
> 
> Do you want to do that with this patch, or do you want to get this change in?

Regarding the simplification, I just opened bug 137927. If we need to debug the code just to figure it out, and given our platforms' different approaches to accessible text, tackling that problem should be separated out.

As for getting the change from the attached patch in, I'm comfortable with it for my platform if you're comfortable with it for yours. If you'd prefer I make the change in the ATK code or write some more tests so we can be sure it isn't breaking anything on the Mac, I'm happy to do so.
Comment 9 WebKit Commit Bot 2014-10-21 11:44:19 PDT
Comment on attachment 239742 [details]
Patch

Clearing flags on attachment: 239742

Committed r174992: <http://trac.webkit.org/changeset/174992>
Comment 10 WebKit Commit Bot 2014-10-21 11:44:23 PDT
All reviewed patches have been landed.  Closing bug.