Closed Bug 1269369 Opened 4 years ago Closed 4 years ago

[e10s] DOMNodeId accessible proxy is not implemented on Windows.

Categories

(Core :: Disability Access APIs, defect)

All
Windows
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
e10s + ---
firefox53 --- fixed

People

(Reporter: yzen, Assigned: yzen)

References

(Blocks 1 open bug)

Details

(Whiteboard: aes+)

Attachments

(1 file, 3 obsolete files)

It looks like document loaded does not fire when a document is loaded in test context with e10s. It will however fire if remote browser is explicitly reloaded.
The tests that fail are from bug 1186029 and a sample logs with and without e10s are here:
https://pastebin.mozilla.org/8869231
https://pastebin.mozilla.org/8869230
tracking-e10s: --- → ?
This is an issue when testing my a11y+e10s builds.
David, is there somebody on the a11y team who could look into this?
Flags: needinfo?(dbolter)
Trevor any hunches?
Yura it looks like your logs expired... can you get fresh ones?
Flags: needinfo?(yzenevich)
Flags: needinfo?(tbsaunde+mozbugs)
Here's a sample: https://pastebin.mozilla.org/8913239

Essentially the test just times out because it only starts when doc loaded event (a11y one) comes from the content process. This however never happens, and from what I remember, when talking to Trevor, it's because it fires too early and is never processed in chrome process.
Flags: needinfo?(yzenevich)
Did more investigation. It looks like document in content process is loaded (DOM event) much earlier than the moment a11y is started in the content process. Aaron, perhaps you'd know (off the top of your head) of a reason this only happens on windows: e.g. document in content process loaded before a11y is initialized?
Flags: needinfo?(aklotz)
I haven't looked much into that, I'm afraid. Most of my stuff is lazily initialized upon the first instantiation of a DocAccessibleChild.
Flags: needinfo?(aklotz)
(In reply to Yura Zenevich [:yzen] from comment #5)
> Did more investigation. It looks like document in content process is loaded
> (DOM event) much earlier than the moment a11y is started in the content
> process. Aaron, perhaps you'd know (off the top of your head) of a reason
> this only happens on windows: e.g. document in content process loaded before
> a11y is initialized?

Shouldn't ApplicationAccessible::Init() make that work anyway?
Flags: needinfo?(tbsaunde+mozbugs)
(In reply to Trevor Saunders (:tbsaunde) from comment #7)
> (In reply to Yura Zenevich [:yzen] from comment #5)
> > Did more investigation. It looks like document in content process is loaded
> > (DOM event) much earlier than the moment a11y is started in the content
> > process. Aaron, perhaps you'd know (off the top of your head) of a reason
> > this only happens on windows: e.g. document in content process loaded before
> > a11y is initialized?
> 
> Shouldn't ApplicationAccessible::Init() make that work anyway?

This doesn't work with e10s in content process, does it? We can't actually enumerate windows when ApplicationAccessible is initialized in content.
(In reply to Yura Zenevich [:yzen] from comment #8)
> (In reply to Trevor Saunders (:tbsaunde) from comment #7)
> > (In reply to Yura Zenevich [:yzen] from comment #5)
> > > Did more investigation. It looks like document in content process is loaded
> > > (DOM event) much earlier than the moment a11y is started in the content
> > > process. Aaron, perhaps you'd know (off the top of your head) of a reason
> > > this only happens on windows: e.g. document in content process loaded before
> > > a11y is initialized?
> > 
> > Shouldn't ApplicationAccessible::Init() make that work anyway?
> 
> This doesn't work with e10s in content process, does it? We can't actually
> enumerate windows when ApplicationAccessible is initialized in content.

duh yeah.

That said I think we do need something similar eventually to handle a11y being enabled after session startup.

I'm not really sure why this happens though.  It might be interesting to debug how / when we start the child process on linux / mac and how it happens on windows and compare those.
I'll take over the investigation of this.
Assignee: nobody → aklotz
Flags: needinfo?(dbolter)
Quick note on this, DOM node ID property is not implemented for proxy accessibles which is part of the problem, even if the events were firing, the test machinery would not recognize that the doc load event is for the expected document.
Attached patch 1269369 wip (obsolete) — Splinter Review
In case this is helpful (slightly out of date as there's a more useful method nsGlobalWindow method - isTopLevelWindow or something like that). Olli suggested, since in content process windowEnumerator will not give us much to use nsGlobalWindow for these purposes.
Whiteboard: aes+
Attached patch rebased wip (obsolete) — Splinter Review
Attachment #8798887 - Attachment is obsolete: true
I did tweak the wip to use IsTopLevelWindow.
Assignee: aklotz → yzenevich
just ni'ing Trevor regardin DOMNodeId stuff for a proxy on Windows.
Flags: needinfo?(tbsaunde+mozbugs)
Summary: [e10s] Document loaded event does not fire on windows when uri is loaded. → [e10s] DOMNodeId accessible proxy is not implemented on Windows.
When tests are enabled we should uplift to 52, as per RyanVM
(In reply to Yura Zenevich [:yzen] from comment #15)
> just ni'ing Trevor regardin DOMNodeId stuff for a proxy on Windows.

ok, it should be pretty easy to do that on top of the patches in bug 1301148, can you handle that?  And the rest of the com interface stuff ideally?
Flags: needinfo?(tbsaunde+mozbugs)
Depends on: 1301148
Attached patch DOMNodeID implementation (obsolete) — Splinter Review
This depends on IGeckoCustom stuff landing first.
Attachment #8806803 - Attachment is obsolete: true
Comment on attachment 8814163 [details] [diff] [review]
DOMNodeID implementation

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

I guess it won't hurt to get a review on this one (still depends on IGeckoCustom  stuff landing.
Attachment #8814163 - Flags: review?(tbsaunde+mozbugs)
What's the status of this? I have a sneaking suspicion that bug 1325244 is being caused by the fact that we don't have top-level DocAccessibles immediately after a11y is started.
Flags: needinfo?(yzenevich)
(In reply to Aaron Klotz [:aklotz] from comment #20)
> What's the status of this? I have a sneaking suspicion that bug 1325244 is
> being caused by the fact that we don't have top-level DocAccessibles
> immediately after a11y is started.

OK, I am no longer sure that this is the case. But I'd still like to know where we stand with this bug :-)
This is blocked by bug 1301148 ATM, but then I should be able to enable the tests (with the patch attached to this bug) and see where we stand in terms of the e10s browser tests.
Flags: needinfo?(yzenevich)
Attachment #8814163 - Flags: review?(tbsaunde+mozbugs) → review+
Depends on: 1329697
Attached patch 1269369 rebasedSplinter Review
Carrying over the r+
Attachment #8814163 - Attachment is obsolete: true
Attachment #8825091 - Flags: review+
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1f076f55d8c
add support for GetID XPCOM method on Windows with e10s. r=tbsaunde
Yura, I think this bug has been overloaded a bit.

From your perspective, you are mainly concerned about automated tests. OTOH, from the user's perspective, we still have a problem: If there are already documents loaded in an e10s browser before a11y is instantiated, those content accessibles are not visible to the AT after a11y instantiation until the previously existing documents have been reloaded. I think that your obsoleted patch in attachment https://bugzilla.mozilla.org/attachment.cgi?id=8798887 helped with this.

What should we do here? Should that work be forked off to a separate bug?
Flags: needinfo?(yzenevich)
This should be a separate bug, I believe. In fact, it should not be too difficult to make up a test case once the browser tests can run on Windows (once bug 1329697 is taken care of). I'll leave ni? to make sure I open the bug for the remaining issue.
https://hg.mozilla.org/mozilla-central/rev/c1f076f55d8c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Flags: needinfo?(yzenevich)
You need to log in before you can comment on or make changes to this bug.