1187740 - Proxy handling for mozHTMLAccessible

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Comment 1

[Lorien Hu (:lsocks)]

Attachment: handle proxies in mozHTMLAccessible

Also removed some trailing whitespace.

GetLevelInternal function is modelled off the Accessible one.

Comment 2

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8639101 [details] [diff] [review]
handle proxies in mozHTMLAccessible

>-    if (parent && parent->Role() == roles::TREE_TABLE) 
>+    if (parent && parent->Role() == roles::TREE_TABLE)

It'd be nice if you kept whitespace changes separate.

> bool
>+DocAccessibleChild::RecvIsHyperText(const uint64_t& aID, bool* aRetVal)

I'd rather not have this, and as far as I can tell you don't use it for anything.

> 
>+bool
>+ProxyAccessible::IsHyperText() const
>+{

same

> int32_t
>+ProxyAccessible::GetLevelInternal()
>+{

how are the over rides of GetLevleInternal supposed to work?  it seems to me it would be simplest to just add a message to get the level.

> - (id)value
> {
>+  uint32_t level = 0;
>   AccessibleWrap* accWrap = [self getGeckoAccessible];
>+  if (accWrap && accWrap->IsHyperText())
>+    level = accWrap->AsHyperText()->GetLevelInternal();

the AsHyperText() is useless, and I'm not sure what the point of the IsHyperText() call is either.

>-- (void)accessibilityPerformAction:(NSString*)action 
>+- (void)accessibilityPerformAction:(NSString*)action
> {
>-  AccessibleWrap* accWrap = [self getGeckoAccessible];
>+  if ([action isEqualToString:NSAccessibilityPressAction]) {
>+    if (AccessibleWrap* accWrap = [self getGeckoAccessible])
>+      accWrap->DoAction(0);
>+    else if (ProxyAccessible* proxy = [self getProxyAccessible])
>+      proxy->DoAction(0);
>+    else
>+      return;

seems like it might be simpler to make that unconditional, then you wouldn't need either else.

> - (NSURL*)url
> {
>-  if (![self getGeckoAccessible] || [self getGeckoAccessible]->IsDefunct())
>-    return nil;
>+  nsAutoString value;
>+  if (AccessibleWrap* accWrap = [self getGeckoAccessible]) {
>+    if (accWrap->IsDefunct())
>+      return nil;

it should never be defunct so that seems kind of silly.

> 
>-  nsAutoString value;
>-  [self getGeckoAccessible]->Value(value);
>+    accWrap->Value(value);
>+  } else if (ProxyAccessible* proxy = [self getProxyAccessible]) {
>+    if (proxy->State() & states::DEFUNCT)
>+      return nil;

same just drop it

Comment 3

[Lorien Hu (:lsocks)]

Attachment: handle proxies in mozHTMLAccessible with fixes

Comment 4

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8643931 [details] [diff] [review]
handle proxies in mozHTMLAccessible with fixes

the random whitespace changes are kind of annoying, please put those in their own patch.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4b5caf0208e

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b4b5caf0208e