Closed
Bug 1124449
Opened 10 years ago
Closed 10 years ago
teach IAccessible impl about proxy wrappers
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla38
Tracking | Status | |
---|---|---|
firefox38 | --- | fixed |
People
(Reporter: tbsaunde, Assigned: tbsaunde)
References
Details
Attachments
(1 file, 1 obsolete file)
17.83 KB,
patch
|
davidb
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8556574 -
Flags: review?(dbolter)
Comment 2•10 years ago
|
||
Comment on attachment 8556574 [details] [diff] [review]
teach IAccessible impl about proxy wrappers
Review of attachment 8556574 [details] [diff] [review]:
-----------------------------------------------------------------
r=me with inlines. Also I'd be more comfortable having TODO comments for the E_NOTIMPL blocks; with bonus marks if they refer to filed bug numbers. BTW I didn't know (or forgot) we had to worry about negative child IDs (?)
::: accessible/ipc/ProxyAccessible.cpp
@@ +51,5 @@
> }
>
> +bool
> +ProxyAccessible::MustPruneChildren() const
> +{
Cut and paste code is always hard to swallow but I'm not sure what else you could do here. Maybe adding a comment that this should be kept in sync with sister function would be good.
::: accessible/windows/msaa/AccessibleWrap.cpp
@@ +405,5 @@
>
> + a11y::role geckoRole;
> + if (IsProxy()) {
> + geckoRole = Proxy()->Role();
> +} else {
The above line needs to be indented.
@@ +413,4 @@
> #endif
>
> + geckoRole = xpAccessible->Role();
> +}
The } needs to be indented.
@@ +453,5 @@
> return S_OK;
> }
>
> +if (IsProxy())
> + return E_FAIL;
Please indent the above two lines.
Attachment #8556574 -
Flags: review?(dbolter) → review+
Assignee | ||
Comment 3•10 years ago
|
||
fixed up a couple things
Attachment #8556574 -
Attachment is obsolete: true
Attachment #8561598 -
Flags: review?(dbolter)
Comment 4•10 years ago
|
||
Comment on attachment 8561598 [details] [diff] [review]
teach IAccessible impl about proxy wrappers
Review of attachment 8561598 [details] [diff] [review]:
-----------------------------------------------------------------
thanks.
::: accessible/ipc/ProxyAccessible.cpp
@@ +52,5 @@
>
> +bool
> +ProxyAccessible::MustPruneChildren() const
> +{
> + // this is the equivelent to nsAccUtils::MustPrune for proxies and should be
nit: "equivelent" should be "equivalent"
Attachment #8561598 -
Flags: review?(dbolter) → review+
Assignee | ||
Comment 5•10 years ago
|
||
Comment 6•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Updated•10 years ago
|
Assignee: nobody → tbsaunde+mozbugs
You need to log in
before you can comment on or make changes to this bug.
Description
•