Closed Bug 1483330 Opened 7 years ago Closed 3 years ago

connect AccessibleNode with Accessible

Categories

(Core :: Disability Access APIs, defect, P5)

defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: dora.tokio, Unassigned, NeedInfo)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.99 Safari/537.36
ARIA attributes are referenced in various places now and I don't know yet how to deal with it uniformly, but boolean properties and some DOMString properties of AccessibleNode are mainly referred to through MapToState. So first I'll try to modify this function.
Attached patch patchSplinter Review
I wrote test-patches, which is limited to MapToState function and also AOMBooleanProperty. I tried to write as uniformly as possible, but I couldn't replace aria_busy part with MapAOMType, the function I defined in this patch, because MapEnumType is used for aria_busy even though MapToketype was used for nother boolean attributes. (I feel changing the type of AccessibleNode::busy from boolean to DOMString would make this situation simpler.) Anyway, how do you think this patch?
Attachment #9000106 - Flags: review?(surkov.alexander)
(In reply to dora.tokio from comment #2) > Anyway, how do you think this patch? It looks good expect you mix up aria and aom badly, which looks confusing. You also should add some magic to ARIARoleMap() method.
Assignee: nobody → dora.tokio
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 9000106 [details] [diff] [review] patch Review of attachment 9000106 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsINode.h @@ +1752,5 @@ > already_AddRefed<mozilla::dom::Promise> > Localize(JSContext* aCx, mozilla::dom::L10nCallback& aCallback, mozilla::ErrorResult& aRv); > > already_AddRefed<mozilla::dom::AccessibleNode> GetAccessibleNode(); > + already_AddRefed<mozilla::dom::AccessibleNode> ExistingAccessibleNode(); just return a raw pointer. Also I think dom requires you to put 'Get' keyword since the method may return null, for example GetAccessibleNodeIfAny or GetAccessibleNodeIfExists
Tokio, do you have ideas how to sort out aria/aom thing, at least make it nicer?
Flags: needinfo?(dora.tokio)
(In reply to alexander :surkov (:asurkov) from comment #5) > Tokio, do you have ideas how to sort out aria/aom thing, at least make it > nicer? This is a difficult problem and I'm not sure whether it would be OK if I keep going because ARIA attributes are referenced in various places/ways. One way to sort out will be adding a method of operation to which eStateRule property corresponds, but there are a lot of tricky operations, such as EnumTypeData, and it would become more confusing. I feel this implementation is a point of compromise for now, admitting confusion though. What part do you feel particularly mixed up?
Flags: needinfo?(dora.tokio)
(In reply to dora.tokio from comment #6) > One way to sort out will be adding a method of operation to which eStateRule > property corresponds, => adding to ARIARoleMap a property indicating a method how to operate eStateRule
(In reply to dora.tokio from comment #6) > (In reply to alexander :surkov (:asurkov) from comment #5) > > Tokio, do you have ideas how to sort out aria/aom thing, at least make it > > nicer? > > This is a difficult problem and I'm not sure whether it would be OK if I > keep going because ARIA attributes are referenced in various places/ways. > One way to sort out will be adding a method of operation to which eStateRule > property corresponds, but there are a lot of tricky operations, such as > EnumTypeData, and it would become more confusing. > I feel this implementation is a point of compromise for now, admitting > confusion though. What part do you feel particularly mixed up? Technically you call a method of aria namespace aria::MapToState, which calls MapAOMType, what makes AOM and then checks for ARIA attributes, which looks confusing. I think your approach is good on concept level, but it should be more accurate from naming perspective. So aria::MapToState/GetRoleMap is right entry points. You can keep aria prefix for now to avoid bulky renames, but could you keep MapEnumType instead of confusing MapAOMType and just modify it to make AOM check? It'd be more accurate for naming. And please adjust comments to include AOM bit.
ni? for Tokio to keep the bug on track.
Flags: needinfo?(dora.tokio)
Comment on attachment 9000106 [details] [diff] [review] patch Review of attachment 9000106 [details] [diff] [review]: ----------------------------------------------------------------- cancelling request until comment 8 is addressed
Attachment #9000106 - Flags: review?(surkov.alexander)
Priority: -- → P5

The bug assignee didn't login in Bugzilla in the last 7 months.
:Jamie, could you have a look please?
For more information, please visit auto_nag documentation.

Assignee: dora.tokio → nobody
Flags: needinfo?(jteh)

AccessibleNode is from an old version of the AOM spec. It's possible we may need something similar when we get to virtual a11y nodes, but until that is actually specified, we really have no idea. Closing for now.

Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(jteh)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: