There are a number of ways to specify it. I think this should be an AXProgressIndicator, and its label/valuetext should be from the text contents. <meter value=6 max=8>6 blocks used (out of 8 total)</meter> <meter value=0.75><img alt="75%" src="graph75.png"></meter> <meter min="0" max="100" value="75"></meter> <meter min=0 max=20 value=12>12cm</meter> <meter min=0 max=10 value=2>2cm</meter> <meter min=0 max=20 value=12 title="centimeters">12cm</meter> <meter min=0 max=10 value=2 title="centimeters">2cm</meter> http://www.w3.org/html/wg/drafts/html/master/forms.html#the-meter-element Please update the role-subrole-description.html layout test with any fixes for this bug.
<rdar://problem/13658964>
Created attachment 199687 [details] patch
Adding Tim to help with review
Attachment 199687 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/accessibility/meter-element-expected.txt', u'LayoutTests/accessibility/meter-element.html', u'LayoutTests/platform/mac/accessibility/role-subrole-roledescription-expected.txt', u'LayoutTests/platform/mac/accessibility/role-subrole-roledescription.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/accessibility/AXObjectCache.cpp', u'Source/WebCore/accessibility/AccessibilityNodeObject.cpp', u'Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp', u'Source/WebCore/accessibility/AccessibilityProgressIndicator.h', u'Source/WebCore/accessibility/AccessibilityRenderObject.cpp']" exit_code: 1 Source/WebCore/accessibility/AXObjectCache.cpp:364: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:75: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:101: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 3 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 199688 [details] patch
Comment on attachment 199687 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=199687&action=review > Source/WebCore/ChangeLog:5 > + please stick the radar here > Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:65 > + return 0.0f; I think we usually use 0 here. I see you have another 0.0f below, too. > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:228 > + // calculate some internal property (like it's description). no apostrophe
Created attachment 199691 [details] patch patch addressing Tim's comments. I didn't change the 0.0f. It looks like there are a lot of places in the source, when a float is being returned, that 0.0f is used
(In reply to comment #7) > Created an attachment (id=199691) [details] > patch > > patch addressing Tim's comments. > > I didn't change the 0.0f. It looks like there are a lot of places in the source, when a float is being returned, that 0.0f is used WebKit style is actually pretty clear (http://www.webkit.org/coding/coding-style.html): > Unless required in order to force floating point math, do not append .0, .f and .0f to floating point literals. "return" is not a case in which you need to force floating point math, so should not be using any of those suffixes. Odd that this code got away with ignoring this part of the guide for so long. But that's probably something to be fixed later, not in this patch :)
(In reply to comment #8) > (In reply to comment #7) > > Created an attachment (id=199691) [details] [details] > > patch > > > > patch addressing Tim's comments. > > > > I didn't change the 0.0f. It looks like there are a lot of places in the source, when a float is being returned, that 0.0f is used > > WebKit style is actually pretty clear (http://www.webkit.org/coding/coding-style.html): > > > Unless required in order to force floating point math, do not append .0, .f and .0f to floating point literals. > > "return" is not a case in which you need to force floating point math, so should not be using any of those suffixes. Odd that this code got away with ignoring this part of the guide for so long. > > But that's probably something to be fixed later, not in this patch :) Thanks. I'll update this patch
Created attachment 199694 [details] patch
Comment on attachment 199694 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=199694&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:233 > + // If an object can't have children, then it is using this method to help > + // calculate some internal property (like its description). > + // In this case, it should check the Node level for children in case they're > + // not rendered (like a <meter> element). > + if (!firstChild && !canHaveChildren()) > + return AccessibilityNodeObject::firstChild(); > + Is any code anywhere ever going to even come close to assuming that AccessibilityRenderObject::firstChild() is an AccessibilityRenderObject? (and then just static_casting or something?) Can we make sure that it won't?
Comment on attachment 199694 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=199694&action=review >> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:233 >> + > > Is any code anywhere ever going to even come close to assuming that AccessibilityRenderObject::firstChild() is an AccessibilityRenderObject? (and then just static_casting or something?) Can we make sure that it won't? I just went through all the firstChild() usage and all the static_cast<AXRenderObject> usage and did not see anything that casted without checking the type first. I believe the AX code is pretty good about that right now.
(In reply to comment #12) > (From update of attachment 199694 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=199694&action=review > > >> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:233 > >> + > > > > Is any code anywhere ever going to even come close to assuming that AccessibilityRenderObject::firstChild() is an AccessibilityRenderObject? (and then just static_casting or something?) Can we make sure that it won't? > > I just went through all the firstChild() usage and all the static_cast<AXRenderObject> usage and did not see anything that casted without checking the type first. I believe the AX code is pretty good about that right now. Just slightly worried that we're providing a new and unusual way for this to happen.
(In reply to comment #13) > (In reply to comment #12) > > (From update of attachment 199694 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=199694&action=review > > > > >> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:233 > > >> + > > > > > > Is any code anywhere ever going to even come close to assuming that AccessibilityRenderObject::firstChild() is an AccessibilityRenderObject? (and then just static_casting or something?) Can we make sure that it won't? > > > > I just went through all the firstChild() usage and all the static_cast<AXRenderObject> usage and did not see anything that casted without checking the type first. I believe the AX code is pretty good about that right now. > > Just slightly worried that we're providing a new and unusual way for this to happen. Understand your concern. I think the layout tests give us pretty good coverage over lots of bad cases, but there is some risk especially if future code makes incorrect assumptions
(In reply to comment #14) > (In reply to comment #13) > > (In reply to comment #12) > > > (From update of attachment 199694 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=199694&action=review > > > > > > >> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:233 > > > >> + > > > > > > > > Is any code anywhere ever going to even come close to assuming that AccessibilityRenderObject::firstChild() is an AccessibilityRenderObject? (and then just static_casting or something?) Can we make sure that it won't? > > > > > > I just went through all the firstChild() usage and all the static_cast<AXRenderObject> usage and did not see anything that casted without checking the type first. I believe the AX code is pretty good about that right now. > > > > Just slightly worried that we're providing a new and unusual way for this to happen. > > Understand your concern. I think the layout tests give us pretty good coverage over lots of bad cases, but there is some risk especially if future code makes incorrect assumptions Thanks for the review
Comment on attachment 199694 [details] patch Attachment 199694 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/136592
Created attachment 199730 [details] patch for landing
Attachment 199730 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/accessibility/meter-element-expected.txt', u'LayoutTests/accessibility/meter-element.html', u'LayoutTests/platform/mac/accessibility/role-subrole-roledescription-expected.txt', u'LayoutTests/platform/mac/accessibility/role-subrole-roledescription.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/accessibility/AXObjectCache.cpp', u'Source/WebCore/accessibility/AccessibilityNodeObject.cpp', u'Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp', u'Source/WebCore/accessibility/AccessibilityProgressIndicator.h', u'Source/WebCore/accessibility/AccessibilityRenderObject.cpp']" exit_code: 1 Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:94: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 199730 [details] patch for landing Attachment 199730 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/109221
Created attachment 199747 [details] patch for landing
Comment on attachment 199747 [details] patch for landing Attachment 199747 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/23345
duplicate symbol __ZTVN7WebCore30AccessibilityProgressIndicatorE in: /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/AccessibilityAllInOne.o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/AccessibilityProgressIndicator.o duplicate symbol __ZNK7WebCore30AccessibilityProgressIndicator29computeAccessibilityIsIgnoredEv in: /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/AccessibilityAllInOne.o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/AccessibilityProgressIndicator.o duplicate symbol __ZNK7WebCore30AccessibilityProgressIndicator13valueForRangeEv in: /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/AccessibilityAllInOne.o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/AccessibilityProgressIndicator.o duplicate symbol __ZNK7WebCore30AccessibilityProgressIndicator16maxValueForRangeEv in: /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/AccessibilityAllInOne.o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/AccessibilityProgressIndicator.o duplicate symbol __ZNK7WebCore30AccessibilityProgressIndicator16minValueForRangeEv in: /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/AccessibilityAllInOne.o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/AccessibilityProgressIndicator.o ld: 13 duplicate symbols for architecture x86_64
Comment on attachment 199747 [details] patch for landing Attachment 199747 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/226015
Created attachment 199752 [details] patch for landing I think this is it. Windows only builds AX files if they're in AccessibilityAllInOne.cpp If something is in that file, it can't be part of the compile targets on mac
http://trac.webkit.org/changeset/149155