Use AXValueDescription to describe the status of the value using “Optimal”, “Suboptimal” and “Critical” by aligning with UIA and MSAA see http://www.w3.org/TR/html51/semantics.html#the-meter-element <rdar://problem/26638677>
Created attachment 291976 [details] patch
Comment on attachment 291976 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=291976&action=review > Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:176 > + // Only expose this when the author has explicitly specified following attributes. specified "the" following > Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:184 > + description = "optimum"; I think we can return directly from this switch statement instead of setting a value > Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:190 > + description = "less good"; I think either this method should return the enum or it should return the localized string itself right now it returns an intermediary form which still needs to be processed further
Created attachment 291982 [details] patch updated from review and should've fixed the build failure.
Comment on attachment 291982 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=291982&action=review > Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:175 > + if (!m_renderer || !m_renderer->isMeter()) why do we only want this on COCOA? (does COCOA also mean iOS?)
Comment on attachment 291982 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=291982&action=review >> Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:175 >> + if (!m_renderer || !m_renderer->isMeter()) > > why do we only want this on COCOA? (does COCOA also mean iOS?) Because the localized string is in cocoa, I followed other mac only descriptions. I think it's both mac and iOS. But Meter is not currently supported on iOS. Should we wrap with platform(mac)?
Comment on attachment 291982 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=291982&action=review >>> Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:175 >>> + if (!m_renderer || !m_renderer->isMeter()) >> >> why do we only want this on COCOA? (does COCOA also mean iOS?) > > Because the localized string is in cocoa, I followed other mac only descriptions. I think it's both mac and iOS. But Meter is not currently supported on iOS. Should we wrap with platform(mac)? I think the #if FEATURE(METER) flag would handle that. so maybe we use COCOA and METER > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2620 > + if (builder.length()) is there no better way to do this? does stringBuilder have methods for this kind of scenario? > Source/WebCore/platform/LocalizedStrings.cpp:786 > + if (value == "optimum") I don't think we need to use strings here for the value. It seems like using an enum would be better. less copying required
Comment on attachment 291982 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=291982&action=review Will address other issues. >> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2620 >> + if (builder.length()) > > is there no better way to do this? does stringBuilder have methods for this kind of scenario? I don't think so. I've searched the code, we have static function shouldAddSpaceBeforeAppendingNextElement in AccessibilityNodeObject doing the length check. I think it's easier here than try to expose that function.
Comment on attachment 291982 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=291982&action=review >>>> Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:175 >>>> + if (!m_renderer || !m_renderer->isMeter()) >>> >>> why do we only want this on COCOA? (does COCOA also mean iOS?) >> >> Because the localized string is in cocoa, I followed other mac only descriptions. I think it's both mac and iOS. But Meter is not currently supported on iOS. Should we wrap with platform(mac)? > > I think the #if FEATURE(METER) flag would handle that. so maybe we use COCOA and METER This #if PLATFORM(COCOA) is already in the #if ENABLE(METER_ELEMENT) wrap
Created attachment 292028 [details] patch updated the function to take enum instead
Comment on attachment 292028 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=292028&action=review > Source/WebCore/platform/LocalizedStrings.cpp:788 > +String AXMeterGaugeRegionValueText(int value) shouldn't int -> GaugeRegion
Comment on attachment 292028 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=292028&action=review >> Source/WebCore/platform/LocalizedStrings.cpp:788 >> +String AXMeterGaugeRegionValueText(int value) > > shouldn't int -> GaugeRegion I tried to include HTMLMeterElement.h in LocalizedStrings.h but it always gave me file not found error.
Created attachment 292072 [details] patch Split the function so that we don't need to deal with enum and includes in LocalizedString.h
Comment on attachment 292072 [details] patch Clearing flags on attachment: 292072 Committed r207540: <http://trac.webkit.org/changeset/207540>
All reviewed patches have been landed. Closing bug.