bug 127447 combined "isAccessibilityObject" and "accessibilityIsIgnored", but in hindsight, it'd be good to do these separately. For example, ""<img src="foo" alt=""> still has an accessibility object (and therefore a role) even though the element is ignored in the platform API. Likewise, elements with the presentation role are ignored for the API, but have an object and would be able to indicate more contextual information than just the "No accessibility information" that is conveyed now.
<rdar://problem/16021878>
accessibilityIsIgnored is now just "ignored" in the inspector-protocol for getAccessibilityPropertiesForNode
Calling this "exists" ( was previously combined with ignored), and also adding required and invalid on relevant fields.
Created attachment 224169 [details] patch
Created attachment 224172 [details] patch
Created attachment 224230 [details] patch fixing apply conflict with TOT
Not sure why this patch is not applying.
Seems like your DOMNodeDetailsSidebarPanel.js is out of date.
Or this patch depends on another patch that is also up for review and not in TOT yet.
Created attachment 224329 [details] patch
Created attachment 224330 [details] patch
I'm up-to-date on all files (the diffs appear clean to me), and this patch doesn't depend on anything that's missing from TOT.
This is frustrating. I’m going to check out a clean copy and reapply the diffs manually.
Created attachment 224331 [details] patch minus the file in question Not for submission. Just testing to see if this applies.
Created attachment 224332 [details] patch
Maybe a bug in ./Tools/Scripts/svn-apply ??
Created attachment 224335 [details] patch
That's better. The diffs were identical, but it seems svn-apply wasn't able to handle a regular diff without the extra symbols added by svn-create-patch.
Comment on attachment 224335 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=224335&action=review > Source/WebCore/inspector/protocol/DOM.json:68 > + { "name": "required", "type": "boolean", "description": "Returns whether the element is required or not required." }, > + { "name": "role", "type": "string", "description": "Computed value for first recognized role token, default role per element, or overridden role." }, > + { "name": "supportsRequired", "type": "boolean", "description": "Returns whether the required state is relevant for this element (form fields, etc)." }, "required" can be marked optional. And if it is supported, it can be included. The front-end can check for the existence of required using: "required" in axobj or axobj.required !== undefined. > Source/WebInspectorUI/UserInterface/DOMNodeDetailsSidebarPanel.js:245 > + function accessibilityPropertiesCallback(props) accessibilityProperties is better. We don't abbreviate in WebKit. > Source/WebInspectorUI/UserInterface/DOMNodeDetailsSidebarPanel.js:253 > + var ignored = (props.ignored ? "true" : ""); > + var invalid = (props.invalid && props.invalid !== "false" ? props.invalid : ""); WebKit style drops the parens in these cases. > Source/WebInspectorUI/UserInterface/DOMNodeDetailsSidebarPanel.js:260 > + var required = (props.supportsRequired ? (props.required ? "true" : "false") : ""); No need for the outer parens. > Source/WebInspectorUI/UserInterface/DOMNodeDetailsSidebarPanel.js:276 > + this._accessibilityNodeIgnoredRow.value = ignored; > + this._accessibilityNodeInvalidRow.value = invalid; > + this._accessibilityNodeLabelRow.value = label; > + this._accessibilityNodeRequiredRow.value = required; > + this._accessibilityNodeRoleRow.value = role; ignored and required look like they are "true" and "false" strings. Those user visible strings should be WebInspector.UIStrings. Something like "Yes" and "No" would be nicer and can be localized.
Comment on attachment 224335 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=224335&action=review > Source/WebCore/inspector/InspectorDOMAgent.cpp:1434 > + required = axObject->isRequired(); wrong spacing after "="
(In reply to comment #19) > > Source/WebInspectorUI/UserInterface/DOMNodeDetailsSidebarPanel.js:245 > > + function accessibilityPropertiesCallback(props) > > accessibilityProperties is better. We don't abbreviate in WebKit. I was trying to reduce redundancy as suggested in the previous bug. If "accessibilityProperties" is the only argument of the "accessibilityPropertiesCallback" callback, why should it be so verbose?
Created attachment 224450 [details] patch with review feedback
Comment on attachment 224450 [details] patch with review feedback Actually, I'm going to wrap the block of optional attrs in an "exists" conditional. Marking previous as obsolete.
Created attachment 224453 [details] patch with review feedback
Comment on attachment 224453 [details] patch with review feedback View in context: https://bugs.webkit.org/attachment.cgi?id=224453&action=review > Source/WebCore/inspector/InspectorDOMAgent.cpp:1443 > .setNodeId(pushNodePathToFrontend(node)); > - > + if (exists) { An empty line here helps readability. > Source/WebInspectorUI/UserInterface/DOMNodeDetailsSidebarPanel.js:291 > + ]; > this._accessibilityEmptyRow.hideEmptyMessage(); An empty line here would be nice.
Created attachment 224562 [details] patch
Comment on attachment 224562 [details] patch Clearing flags on attachment: 224562 Committed r164332: <http://trac.webkit.org/changeset/164332>
All reviewed patches have been landed. Closing bug.