[AccessFu] Always use vc of nested iframes

RESOLVED FIXED in mozilla24

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: eeejay, Assigned: eeejay)

Tracking

unspecified
mozilla24
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Currently we only do that with OOP frames. We should always use a nested frame's vc, if available, this will make us work in a more consistent manner across platforms, and catch bugs earlier.
(Assignee)

Updated

5 years ago
Depends on: 872338, 871859
(Assignee)

Updated

5 years ago
Depends on: 857946
No longer depends on: 871859
(Assignee)

Comment 1

5 years ago
Created attachment 749617 [details] [diff] [review]
Always use nested virtual cursors, when available.

The main change in this patch is how we calculate offsets when highlighting objects in nested documents.
Attachment #749617 - Flags: review?(yura.zenevich)
I've tested this patch along with the one for bug 857946 and it looks like the offsets are correct for the elements that I could reach (the top notification/status bar). However, I keep on getting "ERROR TypeError: null has no properties" on line 89 of content-script.js (vc is null in those cases).

As far as I understand this bug might be dependent on bug 872338 since Utils.getVirtualCursor produces an exception when trying to query for Ci.nsIAccessibleCursorable and does not return the virtualCursor value.
(Assignee)

Comment 3

5 years ago
(In reply to Yura Zenevich [:yzen] from comment #2)
> I've tested this patch along with the one for bug 857946 and it looks like
> the offsets are correct for the elements that I could reach (the top
> notification/status bar). However, I keep on getting "ERROR TypeError: null
> has no properties" on line 89 of content-script.js (vc is null in those
> cases).
> 
> As far as I understand this bug might be dependent on bug 872338 since
> Utils.getVirtualCursor produces an exception when trying to query for
> Ci.nsIAccessibleCursorable and does not return the virtualCursor value.

correct. you need the patches from both dependant bugs.
(In reply to Eitan Isaacson [:eeejay] from comment #3)
> correct. you need the patches from both dependant bugs.
I should've checked "Depends On" section :)
(Assignee)

Comment 5

5 years ago
Created attachment 750065 [details] [diff] [review]
Bug 872355 - Always use nested virtual cursors, when available.

Minor tweaks to support removal of nsIAccessibleCursorable
Attachment #749617 - Attachment is obsolete: true
Attachment #749617 - Flags: review?(yura.zenevich)
Attachment #750065 - Flags: review?(yura.zenevich)
(Assignee)

Comment 6

5 years ago
Created attachment 751730 [details] [diff] [review]
Bug 872355 - Always use nested virtual cursors, when available.

This also consolidates translation of coordinates in one place, and adds hi-dpi (retina) support.
Attachment #750065 - Attachment is obsolete: true
Attachment #750065 - Flags: review?(yura.zenevich)
Attachment #751730 - Flags: review?(yura.zenevich)
Comment on attachment 751730 [details] [diff] [review]
Bug 872355 - Always use nested virtual cursors, when available.

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

Looks good. r=me, with a couple of nits.

::: accessible/src/jsat/AccessFu.jsm
@@ +484,2 @@
>  
> +    let scale = 1 / Utils.win.QueryInterface(Ci.nsIInterfaceRequestor)

Looks like a good candidate to be in Utils (Also used in content-script.js). Something like:
getPPCP: function getPPCP(aWin) {
  return aWin.QueryInterface(Ci.nsIInterfaceRequestor)
    .getInterface(Ci.nsIDOMWindowUtils).screenPixelsPerCSSPixel;
},

::: accessible/src/jsat/Utils.jsm
@@ +144,5 @@
>  
>      return messageManagers;
>    },
>  
> +  get isContentProcess() {

NIT: This could just be
return Services.appinfo.processType == Services.appinfo.PROCESS_TYPE_CONTENT;
Attachment #751730 - Flags: review?(yura.zenevich) → review+
https://hg.mozilla.org/mozilla-central/rev/8521a38f2803
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.