Closed Bug 872338 Opened 7 years ago Closed 7 years ago

iframe documents should support nsIAccessibleCursorable too

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: eeejay, Assigned: eeejay)

References

Details

Attachments

(2 files, 1 obsolete file)

Originally we designated nsIAccessibleCursorable to app top-level, and tab documents. This was before we knew what B2G internals would look like.

Now we know that B2G uses out-of-process content in iframes for most of its apps, as well as the homescreen. We naturally support that because an oop content doc is considered top-level, and gets nsIAccessibleCursorable.

The way we currently deal with this, is we support nested virtual cursors, for example, if the user steps in to an app iframe, a message is sent to the doc in that frame, which moves its cursor to the first item. Even thought there is more than one virtual cursor, the user still perceives is as one.

So in the case of OOP apps, if the user explores to a link, the actual vc state is:
root vc: internal frame
internal frame vc: link

This works fine with OOP iframes, but since there are no OOP frames in desktop, we only use the top-level vc.

I would like to use the same code paths for oop and non-oop iframes. This means having documents in iframes support nsIAccessibleCursorable.
Attachment #749607 - Flags: review?(surkov.alexander)
Comment on attachment 749607 [details] [diff] [review]
Make all documents support nsIAccessibleCursorable

>+++ b/accessible/public/nsIAccessibleCursorable.idl
>@@ -5,17 +5,17 @@
>  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> 
> #include "nsISupports.idl"
> 
> interface nsIAccessiblePivot;
> 
> /**
>  * An interface implemented by an accessible object that has an associated
>- * virtual cursor. Typically, a top-level application or content document.
>+ * virtual cursor. Typically, an application or content document.

you should just merge this interface into nsIAccessibleDocument
(In reply to Trevor Saunders (:tbsaunde) from comment #2)
> Comment on attachment 749607 [details] [diff] [review]
> Make all documents support nsIAccessibleCursorable
> 
> >+++ b/accessible/public/nsIAccessibleCursorable.idl
> >@@ -5,17 +5,17 @@
> >  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> > 
> > #include "nsISupports.idl"
> > 
> > interface nsIAccessiblePivot;
> > 
> > /**
> >  * An interface implemented by an accessible object that has an associated
> >- * virtual cursor. Typically, a top-level application or content document.
> >+ * virtual cursor. Typically, an application or content document.
> 
> you should just merge this interface into nsIAccessibleDocument

Maybe. We may want this interface in other kinds of objects in the future.
Blocks: 872355
Comment on attachment 749607 [details] [diff] [review]
Make all documents support nsIAccessibleCursorable

as long as API consumer is happy with changes I don't mind
Attachment #749607 - Flags: review?(surkov.alexander) → review+
(In reply to Eitan Isaacson [:eeejay] from comment #3)

> > >  * An interface implemented by an accessible object that has an associated
> > >- * virtual cursor. Typically, a top-level application or content document.
> > >+ * virtual cursor. Typically, an application or content document.
> > 
> > you should just merge this interface into nsIAccessibleDocument
> 
> Maybe. We may want this interface in other kinds of objects in the future.

# it saves bytes per document, not a big win but anyway
# API less complicated
# if you will need something in the future then you can take care about it in the future. at least until you have concrete ideas or plans
(In reply to alexander :surkov from comment #4)
> Comment on attachment 749607 [details] [diff] [review]
> Make all documents support nsIAccessibleCursorable
> > as long as API consumer is happy with changes I don't mind

I don't mind, but I'm not sure I'm ok with leaving funny stuff lying around incase it may be useful some day.
Removed nsIAccessibleCursorable
Attachment #750062 - Flags: review?(trev.saunders)
Attachment #749607 - Attachment is obsolete: true
Attachment #750062 - Flags: review?(trev.saunders) → review+
Dang, I need to remove some mochitests too. Separate patch coming up.
Attachment #751263 - Flags: review?(trev.saunders) → review+
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/a20b345ae2cb.

It's going to be hard to pick your bustage out from the bustage on the push below you, but eventually https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=989f42c1bc87 (the backout of the one below you) should show it. Windows builds like https://tbpl.mozilla.org/php/getParsedLog.php?id=23103300&tree=Mozilla-Inbound, but I think there's going to also be a bunch of test failures.
I underestimated bz's ability to fail tests in a variety of ways, I think all the test failures were his. Which leaves us with the fascinating situation where Windows opt and debug built fine on your push, builds which didn't claim to be clobbers, and then both failed, neither of them claiming to be clobbers and both on different slaves, on the next push, with "No rule to make target 'nsIAccessibleCursorable.idl' needed by ['<command-line>', 'nsIAccessibleCursorable.idl']". Dunno what to make of that.
Depends on: 873809
(In reply to Phil Ringnalda (:philor) from comment #13)
> Relanding with a touch of /CLOBBER, since bug 873809 makes me think that was
> the only problem.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/9178f40ffa63
> https://hg.mozilla.org/integration/mozilla-inbound/rev/5e792395df36

Thanks for seeing this through!
https://hg.mozilla.org/mozilla-central/rev/5e792395df36
https://hg.mozilla.org/mozilla-central/rev/9178f40ffa63
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.