Closed Bug 872355 Opened 7 years ago Closed 7 years ago

[AccessFu] Always use vc of nested iframes

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: eeejay, Assigned: eeejay)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
Depends on: 872338, 871859
Depends on: 857946
No longer depends on: 871859
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.
(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 :)
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)
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
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.