Closed Bug 1251941 Opened 4 years ago Closed 4 years ago

aria::GetRoleMap should take element

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: surkov, Assigned: surkov)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
a small clean up as we go
Attachment #8724550 - Flags: review?(dbolter)
Assignee: nobody → surkov.alexander
Comment on attachment 8724550 [details] [diff] [review]
patch

Review of attachment 8724550 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/base/nsCoreUtils.cpp
@@ +241,5 @@
>        }
>      }
>    }
>  
> +  return content && content->IsElement() ? content->AsElement() : nullptr;

nit: for easy readability can we put () around the && condition?
Attachment #8724550 - Flags: review?(dbolter) → review+
(In reply to David Bolter [:davidb] from comment #1)

> > +  return content && content->IsElement() ? content->AsElement() : nullptr;
> 
> nit: for easy readability can we put () around the && condition?

next patch this method will go away, however in general I like "a ? b : c" syntax, when expressions are short
(In reply to alexander :surkov from comment #2)
> (In reply to David Bolter [:davidb] from comment #1)
> 
> > > +  return content && content->IsElement() ? content->AsElement() : nullptr;
> > 
> > nit: for easy readability can we put () around the && condition?
> 
> next patch this method will go away, however in general I like "a ? b : c"
> syntax, when expressions are short

but yes, I would
https://hg.mozilla.org/mozilla-central/rev/78d20ae3f50e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.