Bug 117651

Summary: <meter> element text value exposed as AXTitle; should be exposing AXValueDescription
Product: WebKit Reporter: James Craig <jcraig>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, buildbot, cfleizach, commit-queue, dmazzoni, jdiggs, mario, rniwa, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 109023    
Bug Blocks:    
Attachments:
Description Flags
test case
none
Patch for feedback/test results.
none
Patch.
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2
none
Patch.
cfleizach: review+, buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion
none
Patch.
none
Patch.
none
Patch. none

Description James Craig 2013-06-14 12:49:34 PDT
Created attachment 204731 [details]
test case

<meter> element text value exposed as AXTitle; should be exposing AXValueDescription

related to bug 109023 and bug 117650
Comment 1 James Craig 2013-06-14 12:50:52 PDT
<rdar://problem/14159520>
Comment 2 Samuel White 2013-09-06 13:55:03 PDT
Created attachment 210794 [details]
Patch for feedback/test results.

Submitting for feedback and platform test failure results, no review needed.

I'm not sure if we should handle this change at the platform level as implemented, or if we should move to the axObject level so other platforms inherit this behavior. Given the close relationship between the APIs I tend to think the latter is the correct solution. Thoughts? Thanks.
Comment 3 chris fleizach 2013-09-06 13:58:42 PDT
Comment on attachment 210794 [details]
Patch for feedback/test results.

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

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2251
> +        RenderObject* renderer = m_object->renderer();

i think we should add a isMeterElement() method in AccessibilityObject, so we don't have to check the renderer() object in the platform
that will also allow us to remove the ENALBE(METER_ELEMENT) check here. that can just be moved inside isMeterElement()

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2615
> +        RenderObject* renderer = m_object->renderer();

ditto

> LayoutTests/accessibility/meter-element.html:28
> +        debug(meter.valueDescription);

we should probably append "AXValueDescription" in the logging here so that we know what's being output in the expected results
Comment 4 Samuel White 2013-09-09 13:03:08 PDT
Created attachment 211071 [details]
Patch.

Updated test output and added isMeter method as requested. Thanks for taking a look Chris.
Comment 5 Build Bot 2013-09-09 13:31:45 PDT
Comment on attachment 211071 [details]
Patch.

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

New failing tests:
platform/mac/accessibility/aria-valuetext.html
Comment 6 Build Bot 2013-09-09 13:31:47 PDT
Created attachment 211075 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.4
Comment 7 Samuel White 2013-09-09 14:06:11 PDT
Created attachment 211082 [details]
Patch.

Updated WebKit 2 output to fix failures.
Comment 8 Build Bot 2013-09-09 14:56:59 PDT
Comment on attachment 211082 [details]
Patch.

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

New failing tests:
platform/mac/accessibility/aria-valuetext.html
Comment 9 Build Bot 2013-09-09 14:57:02 PDT
Created attachment 211093 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.4
Comment 10 Build Bot 2013-09-09 15:20:45 PDT
Comment on attachment 211082 [details]
Patch.

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

New failing tests:
platform/mac/accessibility/aria-valuetext.html
Comment 11 Build Bot 2013-09-09 15:20:48 PDT
Created attachment 211103 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.4
Comment 12 Build Bot 2013-09-09 15:59:19 PDT
Comment on attachment 211082 [details]
Patch.

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

New failing tests:
platform/mac/accessibility/aria-valuetext.html
Comment 13 Build Bot 2013-09-09 15:59:22 PDT
Created attachment 211109 [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 14 Samuel White 2013-09-09 21:04:14 PDT
Created attachment 211149 [details]
Patch.

Missed one layout test on the last pass that needed updating. Should be good now.
Comment 15 Samuel White 2013-09-09 21:38:49 PDT
Created attachment 211155 [details]
Patch.

Deleted a tab character that snuck in somehow.
Comment 16 Mario Sanchez Prada 2013-09-10 02:57:33 PDT
Comment on attachment 211155 [details]
Patch.

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

> Source/WebCore/accessibility/AccessibilityObject.cpp:545
> +    return false;

We will get a warning (that might be treated as an error) in some platforms if you don't put this return inside an #else branch, due to it being an unreachable statement when METER_ELEMENT is enabled.
Comment 17 Samuel White 2013-09-10 14:44:37 PDT
Created attachment 211246 [details]
Patch.
Comment 18 Samuel White 2013-09-10 14:47:16 PDT
(In reply to comment #16)
> (From update of attachment 211155 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=211155&action=review
> 
> > Source/WebCore/accessibility/AccessibilityObject.cpp:545
> > +    return false;
> 
> We will get a warning (that might be treated as an error) in some platforms if you don't put this return inside an #else branch, due to it being an unreachable statement when METER_ELEMENT is enabled.

Thanks for the tip.
Comment 19 Mario Sanchez Prada 2013-09-12 03:00:11 PDT
Comment on attachment 211246 [details]
Patch.

Looks good to me.
Comment 20 WebKit Commit Bot 2013-09-12 03:23:55 PDT
Comment on attachment 211246 [details]
Patch.

Clearing flags on attachment: 211246

Committed r155603: <http://trac.webkit.org/changeset/155603>
Comment 21 WebKit Commit Bot 2013-09-12 03:23:59 PDT
All reviewed patches have been landed.  Closing bug.