This is for some follow up from discussion on bug 1149772. Vendors are asking. Shall we make the emulation work in e10s mode? Ultimately this is probably the smoothest option for our users...

to make the user experience, either Firefox has to be running in single process mode or emulation has to work in mutli process. Transitional period may be quite long, so perhaps the emulation is something we should do. Is it resource heavy to implement it?

I think we all agree. Changing summary. <@tbsaunde> davidb: I expect we have to <~davidb> tbsaunde, yeah I think you're right

Yura is good to take this.

Comment on attachment 8792538 [details] [diff] [review] 1206711 part 1 Review of attachment 8792538 [details] [diff] [review]: ----------------------------------------------------------------- r=me sorry it got that long ::: accessible/windows/msaa/DocAccessibleWrap.cpp @@ -148,5 @@ > - if (tabChild) > - tabChild->SendGetWidgetNativeData(&nativeData); > - else > - nativeData = reinterpret_cast<mozilla::WindowsHandle>( > - rootDocument->GetNativeWindow()); ideally this should be landed separately ::: accessible/windows/msaa/nsWinUtils.h @@ +14,5 @@ > namespace a11y { > class DocAccessible; > const LPCWSTR kClassNameRoot = L"MozillaUIWindowClass"; > const LPCWSTR kClassNameTabContent = L"MozillaContentWindowClass"; > +const LPCWSTR kPropNameDocAcc = L"DocAccessible"; perhaps we should give it a Moz prefix, to avoid possibly conflicts with somebody trying to set up same property on our window

Pushed by yura.zenevich@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/fca6bd5a5b5b remove unused ipc bits from DocAccessibleWrap::DoInitialUpdate. r=surkov https://hg.mozilla.org/integration/mozilla-inbound/rev/ed4462900a9b store doc accessible in emulated HWND itself. r=surkov

As per IRC this is now our main blocker for e10s + a11y testing on Jaws/WE/Dolphin.

https://hg.mozilla.org/mozilla-central/rev/fca6bd5a5b5b https://hg.mozilla.org/mozilla-central/rev/ed4462900a9b

Comment on attachment 8793883 [details] [diff] [review] 1206711 part 2 I haven't thought about this in depth yet. >diff --git a/accessible/base/Platform.h b/accessible/base/Platform.h >index 2520456..ef9ad03 100644 >--- a/accessible/base/Platform.h >+++ b/accessible/base/Platform.h I'm really not a fan of sticking all these functions specific to one thing in a generic spot and then stubbing them for all the other platforms. > /** >+ * Determine if a given native window is emulated. >+ */ >+bool IsWindowEmulated(void* aWindowHandle); I don't know what you need this for, but it seems like it could be kept in windows/ somehow. >+ >+/** >+ * Depending on the platform and if window emulation is required, create a >+ * native window for a tab accessible document. >+ */ >+void* MaybeCreateNativeWindow(DocAccessibleParent* aDocument); it seems like you could do this in ProxyCreated() if the proxy is a top level document. >+ >+/** >+ * Depending on the platform and if window emulation is enabled, destroy a >+ * native window for a tab accessible document. >+ */ >+void* MaybeDestroyNativeWindow(void* aWindowHandle); same with ProxyDestroyed() and I don't know what the return value is about. it seems like you could split out the creation and destruction out, and get that in as soon as you do it in the existing proxyCreated/Destroyed. >+ >+/** >+ * Update native window parent for emulated window if it is different. >+ */ >+void MaybeUpdateNativeWindowParent(void* aWindowHandle, >+ void* aNewParentWindowHandle); I'm not sure what the best way to handle this is, but probably a static function on AccessibleWrap or maybe just stick the code in TabParent by the code to send down a new window. >index 6f4527a7..4ec9824 100644 >--- a/accessible/other/Platform.cpp >+++ b/accessible/other/Platform.cpp if you add things here you also need to add them for mac and atk. >+bool >+a11y::IsWindowEmulated(void*) >+{ you should return something here, or release assert >+} >+ >+void* >+a11y::MaybeCreateNativeWindow(DocAccessibleParent*) >+{ >+} same >+ >+void* >+a11y::MaybeDestroyNativeWindow(void*) >+{ >+} and heree >+ >+void >+a11y::MaybeUpdateNativeWindowParent(void*, void*) >+{ >+} same > AccessibleWrap::GetHWNDFor(Accessible* aAccessible) > { > if (!aAccessible) { > return nullptr; > } > >- if (XRE_IsContentProcess()) { >- DocAccessible* doc = aAccessible->Document(); >- if (!doc) { >- return nullptr; >- } >- >- DocAccessibleChild* ipcDoc = doc->IPCDoc(); >- if (!ipcDoc) { >- return nullptr; >- } >- >- auto tab = static_cast<dom::TabChild*>(ipcDoc->Manager()); >- MOZ_ASSERT(tab); >- return reinterpret_cast<HWND>(tab->GetNativeWindowHandle()); >- } this doesn't seem right. > DocAccessibleWrap::GetNativeWindow() const > { >- return mHWND ? mHWND : DocAccessible::GetNativeWindow(); >+ if (mHWND) { >+ return mHWND; >+ } >+ if (XRE_IsContentProcess()) { >+ DocAccessibleChild* ipcDoc = IPCDoc(); >+ if (!ipcDoc) { I don't think that should ever be false. >+ return nullptr; >+ } >+ auto tab = static_cast<dom::TabChild*>(ipcDoc->Manager()); >+ MOZ_ASSERT(tab); >+ return reinterpret_cast<HWND>(tab->GetNativeWindowHandle()); static_cast should be fine. >+extern const wchar_t* kPropNameTabContent; why is this extern? and why don't you define it here? >+bool >+a11y::IsWindowEmulated(void* aWindowHandle) >+{ >+ MOZ_ASSERT(aWindowHandle); >+ >+ if (!aWindowHandle) { >+ return false; >+ } >+ HWND hwnd = reinterpret_cast<HWND>(aWindowHandle); >+ return nsWinUtils::IsWindowEmulationStarted() && >+ ::GetPropW(hwnd, kPropNameTabContent); then this is it emulated for a remote tab. > const wchar_t* kPropNameTabContent = L"AccessibleTabWindow"; >@@ -148,28 +150,37 @@ WindowProc(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) > > switch (msg) { > case WM_GETOBJECT: > { > // Do explicit casting to make it working on 64bit systems (see bug 649236 > // for details). > int32_t objId = static_cast<DWORD>(lParam); > if (objId == OBJID_CLIENT) { >+ IAccessible* msaaAccessible = nullptr; > DocAccessible* document = > reinterpret_cast<DocAccessible*>(::GetPropW(hWnd, kPropNameDocAcc)); > if (document) { >- IAccessible* msaaAccessible = nullptr; > document->GetNativeInterface((void**)&msaaAccessible); // does an addref >- if (msaaAccessible) { >- LRESULT result = ::LresultFromObject(IID_IAccessible, wParam, >- msaaAccessible); // does an addref >- msaaAccessible->Release(); // release extra addref >- return result; >+ } else { >+ DocAccessibleParent* docParent = >+ reinterpret_cast<DocAccessibleParent*>( static_cast >+ ::GetPropW(hWnd, kPropNameDocAccParent)); you could use one attribute if you steal a bit from the pointer to store if its remote, but I don't know that it matters. >+ if (docParent) { >+ auto wrapper = >+ reinterpret_cast<DocProxyAccessibleWrap*>(WrapperFor(docParent)); static_cast > #include "mozilla/dom/indexedDB/ActorsParent.h" > #include "mozilla/plugins/PluginWidgetParent.h" >@@ -297,16 +300,19 @@ TabParent::TabParent(nsIContentParent* aManager, > , mTabSetsCursor(false) > , mHasContentOpener(false) > #ifdef DEBUG > , mActiveSupressDisplayportCount(0) > #endif > , mLayerTreeEpoch(0) > , mPreserveLayers(false) > , mFirstActivate(true) >+#if defined(XP_WIN) && defined(ACCESSIBILITY) >+ , mHwnd(nullptr) this should have a name that makes it clear what its about. >- Unused << SendUpdateNativeWindowHandle(newWindowHandle); >+ if (mHwnd && newWindowHandle) { >+ a11y::MaybeUpdateNativeWindowParent( >+ mHwnd, reinterpret_cast<HWND>(newWindowHandle)); >+ } else { >+ Unused << SendUpdateNativeWindowHandle(newWindowHandle); probably better to always send the update even if nothing currently uses it. > #ifdef ACCESSIBILITY >- delete static_cast<a11y::DocAccessibleParent*>(aParent); >+ auto doc = static_cast<a11y::DocAccessibleParent*>(aParent); >+#ifdef XP_WIN >+ if (doc->IsTopLevel() && mHwnd) { >+ mHwnd = reinterpret_cast<HWND>(a11y::MaybeDestroyNativeWindow(mHwnd)); >+ if (!mIsDestroyed) { >+ uintptr_t newWindowHandle = 0; >+ if (nsCOMPtr<nsIWidget> widget = GetWidget()) { >+ newWindowHandle = >+ reinterpret_cast<uintptr_t>(widget->GetNativeData(NS_NATIVE_WINDOW)); >+ } >+ Unused << SendUpdateNativeWindowHandle(newWindowHandle); this seems dangerous, this function can be called while the tab it is in is being torn down. > doc->SetTopLevel(); > a11y::DocManager::RemoteDocAdded(doc); >+#ifdef XP_WIN >+ if (mHwnd) { >+ mHwnd = reinterpret_cast<HWND>(a11y::MaybeDestroyNativeWindow(mHwnd)); and this #ifdefery makes the Platform.h changes make less sense. your basically pretending that its platform independent when basically none of it actually is.

This should updates most of the comments. I changed reinterpret cast to static were possible (there was at least 1 place that you mentioned where static cast was not allowed).

FYI this is still crashing but I managed to get 2 different stack traces before VS debugger crashed: https://pastebin.mozilla.org/8913299 https://pastebin.mozilla.org/8913295

Comment on attachment 8795011 [details] [diff] [review] 1206711 part 2 v2 >+ void SetEmulatedWindowHandle(uintptr_t aWindowHandle) >+ { >+ if (aWindowHandle) { >+ SendEmulatedWindowHandleCreated(aWindowHandle); >+ } else if (mEmulatedWindowHandle) { >+ HWND hWnd = reinterpret_cast<HWND>(mEmulatedWindowHandle); >+ ::DestroyWindow(hWnd); >+ } >+ mEmulatedWindowHandle = aWindowHandle; >+ } >+ >+ void UpdateEmulatedWindowHandleParent(uintptr_t aWindowHandle) { >+ if (mEmulatedWindowHandle && aWindowHandle) { >+ HWND hWnd = reinterpret_cast<HWND>(mEmulatedWindowHandle); >+ HWND parentHwnd = reinterpret_cast<HWND>(aWindowHandle); >+ if (parentHwnd != ::GetParent(hWnd)) { >+ ::SetParent(hWnd, parentHwnd); >+ } >+ } >+ } these are kind of big to put inline, though each caller is doing a destinct thing, so I'd be tempted to just have an API to get / set the handle and do whatever you need in the callers. >+ virtual bool RecvEmulatedWindowHandleCreated(const uintptr_t& aWindowHandle) override; Sorry, what was I thinking when I suggested this? You can just stick this in the SendCOMProxy message which the child blocks on before continueing. So if you setup the emulated window in the content process when that returns you know that it can't possibly matter what the window is yet, and that you will have setup the window handle before it can matter. >+ * Update the child with the tab's current emulated window handle. >+ * >+ * @param aNewHandle The emulated window handle for the tab. given above you can nix this message. > AccessibleWrap::GetHWNDFor(Accessible* aAccessible) > { > if (!aAccessible) { > return nullptr; > } > >- if (XRE_IsContentProcess()) { >- DocAccessible* doc = aAccessible->Document(); >- if (!doc) { >- return nullptr; >- } >- >- DocAccessibleChild* ipcDoc = doc->IPCDoc(); >- if (!ipcDoc) { >- return nullptr; >- } >- >- auto tab = static_cast<dom::TabChild*>(ipcDoc->Manager()); >- MOZ_ASSERT(tab); >- return reinterpret_cast<HWND>(tab->GetNativeWindowHandle()); >- } This doesn't immediately seem right, I expect you good break out some parts of this that can go in on there own, and are easy to make ok. > AccessibleWrap::UpdateSystemCaretFor(Accessible* aAccessible) > { >- // Move the system caret so that Windows Tablet Edition and tradional ATs with >+ // Move the system caret so that Windows Tablet Edition and tradional ATs with please put whitespace / spelling in separate commits, and really I don't see a reason that sort of thing needs to be reviewed. > DocAccessibleWrap::GetNativeWindow() const > { >- return mHWND ? mHWND : DocAccessible::GetNativeWindow(); >+ if (XRE_IsContentProcess()) { >+ DocAccessibleChild* ipcDoc = IPCDoc(); >+ HWND hWnd = reinterpret_cast<HWND>(ipcDoc->GetEmulatedWindowHandle()); >+ if (hWnd) { >+ return hWnd; >+ } >+ >+ auto tab = static_cast<dom::TabChild*>(ipcDoc->Manager()); >+ MOZ_ASSERT(tab); >+ return reinterpret_cast<HWND>(tab->GetNativeWindowHandle()); >+ } else if (mHWND) { >+ return mHWND; >+ } >+ return DocAccessible::GetNativeWindow(); off hand it seems like this will only change the window the document is in, but I haven't looked closely, sorry trying to get something done here. that's it for now, I'll finish off later.

> >- if (XRE_IsContentProcess()) { > >- DocAccessible* doc = aAccessible->Document(); > >- if (!doc) { > >- return nullptr; > >- } > >- > >- DocAccessibleChild* ipcDoc = doc->IPCDoc(); > >- if (!ipcDoc) { > >- return nullptr; > >- } > >- > >- auto tab = static_cast<dom::TabChild*>(ipcDoc->Manager()); > >- MOZ_ASSERT(tab); > >- return reinterpret_cast<HWND>(tab->GetNativeWindowHandle()); > >- } > > This doesn't immediately seem right, I expect you good break out some parts > of this that can go in on there own, and are easy to make ok. > > DocAccessibleWrap::GetNativeWindow() const > > { > >- return mHWND ? mHWND : DocAccessible::GetNativeWindow(); > >+ if (XRE_IsContentProcess()) { > >+ DocAccessibleChild* ipcDoc = IPCDoc(); > >+ HWND hWnd = reinterpret_cast<HWND>(ipcDoc->GetEmulatedWindowHandle()); > >+ if (hWnd) { > >+ return hWnd; > >+ } > >+ > >+ auto tab = static_cast<dom::TabChild*>(ipcDoc->Manager()); > >+ MOZ_ASSERT(tab); > >+ return reinterpret_cast<HWND>(tab->GetNativeWindowHandle()); > >+ } else if (mHWND) { > >+ return mHWND; > >+ } > >+ return DocAccessible::GetNativeWindow(); > > off hand it seems like this will only change the window the document is in, > but I haven't looked closely, sorry trying to get something done here. oh, I see this is just reorganizing, but I don't see any reason you need to do that, and it makes the patch harder to understand, so please undo it.

This is a separated out bit that causes some confusion. GetHWND method in accessible wrap would still call document(Wrap)->GetNativeWindow so there will not be only one way of getting a native window.

Haven't actually tested it yet as trunk build seems to now just crash in my VM but I addressed (I think) most of the things you mentioned.

Comment on attachment 8796296 [details] [diff] [review] 1206711 part 2 Aaron, since you added the original bit for content process in GetHwnd, just wanted to make sure that this looks good to you.

Comment on attachment 8796296 [details] [diff] [review] 1206711 part 2 Review of attachment 8796296 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/windows/msaa/AccessibleWrap.cpp @@ -1301,5 @@ > { > if (!aAccessible) { > return nullptr; > } > - if (XRE_IsContentProcess()) { It is unclear to me why this code was removed; we need to be able to resolve the HWND for any Accessible in content. ::: accessible/windows/msaa/DocAccessibleWrap.cpp @@ +119,5 @@ > // DocAccessible public > void* > DocAccessibleWrap::GetNativeWindow() const > { > + if (XRE_IsContentProcess()) { I would prefer if we just delegated to AccessibleWrap::GetHWNDFor in this case.

I was meaning to say more or less the same thing, but since yzen is out it wasn't terribly high priority. (In reply to Aaron Klotz [:aklotz] from comment #19) > Comment on attachment 8796296 [details] [diff] [review] > 1206711 part 2 > > Review of attachment 8796296 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: accessible/windows/msaa/AccessibleWrap.cpp > @@ -1301,5 @@ > > { > > if (!aAccessible) { > > return nullptr; > > } > > - if (XRE_IsContentProcess()) { > > It is unclear to me why this code was removed; we need to be able to resolve > the HWND for any Accessible in content. I believe it is correctly added in the below hunk, but see below. > ::: accessible/windows/msaa/DocAccessibleWrap.cpp > @@ +119,5 @@ > > // DocAccessible public > > void* > > DocAccessibleWrap::GetNativeWindow() const > > { > > + if (XRE_IsContentProcess()) { > > I would prefer if we just delegated to AccessibleWrap::GetHWNDFor in this > case. I'm not sure if I'm *opposed* to doing this, but without more justification I don't see a reason to do it.

yzen is back \o/

(In reply to Trevor Saunders (:tbsaunde) from comment #20) > I was meaning to say more or less the same thing, but since yzen is out it > wasn't terribly high priority. > > ::: accessible/windows/msaa/DocAccessibleWrap.cpp > > @@ +119,5 @@ > > > // DocAccessible public > > > void* > > > DocAccessibleWrap::GetNativeWindow() const > > > { > > > + if (XRE_IsContentProcess()) { > > > > I would prefer if we just delegated to AccessibleWrap::GetHWNDFor in this > > case. > > I'm not sure if I'm *opposed* to doing this, but without more justification > I don't see a reason to do it. There are other places where GetNativeWindow is used to get a native window other than GetHWNDFor. Since GetHWNDFor also delegates to doc accessible's GetNativeWindow when the argument is not a proxy I was hoping to handle the content process case in GetNativeWindow so it resolves correctly everywhere.

Fair enough. Thanks for the clarification.

See comment 22 why I moved things into doc accessible wrap.

Also part 3 depends on bug 1314707 landing first

(In reply to Yura Zenevich [:yzen] from comment #22) > (In reply to Trevor Saunders (:tbsaunde) from comment #20) > > I was meaning to say more or less the same thing, but since yzen is out it > > wasn't terribly high priority. > > > ::: accessible/windows/msaa/DocAccessibleWrap.cpp > > > @@ +119,5 @@ > > > > // DocAccessible public > > > > void* > > > > DocAccessibleWrap::GetNativeWindow() const > > > > { > > > > + if (XRE_IsContentProcess()) { > > > > > > I would prefer if we just delegated to AccessibleWrap::GetHWNDFor in this > > > case. > > > > I'm not sure if I'm *opposed* to doing this, but without more justification > > I don't see a reason to do it. > > There are other places where GetNativeWindow is used to get a native window > other than GetHWNDFor. Since GetHWNDFor also delegates to doc accessible's > GetNativeWindow when the argument is not a proxy I was hoping to handle the > content process case in GetNativeWindow so it resolves correctly everywhere. It certainly seems like a mistake that AccessibleWrap::GetHWND() and DocAccessible::GetNativeWindow() are different, but I'd tend to say the better thing to do is make everything use GetHWND() and remove GetNativeWindow(). In any case it looks like all the users of GetNativeWindow() either don't need to care about the e10s business, or need to be handled differently anyway. However I guess this isn't a big deal, and this has waited too long (sorry!), so whatever I guess its fine.

Comment on attachment 8814193 [details] [diff] [review] 1206711 part 2 not a fan, but whatever.

Comment on attachment 8814194 [details] [diff] [review] 1206711 part 3 > DocAccessibleChild::DocAccessibleChild(DocAccessible* aDoc) > : DocAccessibleChildBase(aDoc) >+ , mEmulatedWindowHandle(0) ',' on previous line >+ a11y::DocAccessibleParent* doc = GetTopLevelDocAccessible(); >+ HWND hWnd; >+ if (doc && >+ (hWnd = reinterpret_cast<HWND>(doc->GetEmulatedWindowHandle()))) { move the assignment out of the if, just add another if if you must, but I think you can work the check into the next one. > } > > auto parentDoc = static_cast<a11y::DocAccessibleParent*>(aParentDoc); > bool added = parentDoc->AddChildDoc(doc, aParentID); > #ifdef XP_WIN > MOZ_ASSERT(aDocCOMProxy.IsNull()); > if (added) { > a11y::WrapperFor(doc)->SetID(aMsaaID); >+ if (a11y::nsWinUtils::IsWindowEmulationStarted()) { >+ doc->SetEmulatedWindowHandle(parentDoc->GetEmulatedWindowHandle()); I'm not sure we get much from the parent side subdocuments knowing about the fake window, but whatever. > #ifdef XP_WIN > a11y::WrapperFor(doc)->SetID(aMsaaID); > MOZ_ASSERT(!aDocCOMProxy.IsNull()); > RefPtr<IAccessible> proxy(aDocCOMProxy.Get()); > doc->SetCOMProxy(proxy); I'm pretty sure the rest of this needs to happen *before* you call that and send a message to the parent with the doc's fake window. >+ if (a11y::nsWinUtils::IsWindowEmulationStarted()) { >+ a11y::Accessible* outerDoc = doc->OuterDocOfRemoteBrowser(); >+ a11y::RootAccessible* rootDocument = outerDoc->RootAccessible(); >+ MOZ_ASSERT(rootDocument); >+ >+ bool isActive = true; >+ nsIntRect rect(CW_USEDEFAULT, CW_USEDEFAULT, 0, 0); >+ if (a11y::Compatibility::IsDolphin()) { >+ rect = doc->Bounds(); That calls ProxyAccessible::Bounds() which afaik just isn't defined on windows at this point. I guess you could define it using COM, but that's definitely suboptimal perf wise, and may risk deadlocks I'm not sure what happens if the main process COM proxying tries to deadlock with the child process sending the parent a sync ipdl message.

Pushed by yura.zenevich@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/cfba17b7eb3f always use DocAccessibleWrap to get native window. r=tbsaunde

Moving emulation stuff from TabParent to DocAccessibleParent

Comment on attachment 8820057 [details] [diff] [review] 1206711 part 3 ># HG changeset patch ># User Yura Zenevich <yzenevich@mozilla.com> > >Bug 1206711 - adding support for HWND emulation for content top level docs on Windows. s/adding/add/ but really that's not exactly what you are doing here, you stick the hwnd in all docs not just the top level ones. >+DocAccessibleParent::SetEmulatedWindowHandle(uintptr_t aWindowHandle) its ifdefed, you might as well make the type HWND >+{ >+ if (!aWindowHandle && mEmulatedWindowHandle && IsTopLevel()) { I'm not sure there's a good reason to set the member in non top level docs in the first place, but whatever. >+ void SetEmulatedWindowHandle(uintptr_t aWindowHandle); >+ uintptr_t GetEmulatedWindowHandle() const { return mEmulatedWindowHandle; } might as well make the type HWND here too. > nsTArray<DocAccessibleParent*> mChildDocs; > DocAccessibleParent* mParentDoc; > >+#if defined(XP_WIN) >+ // The handle associated with the emulated window that contains this document >+ uintptr_t mEmulatedWindowHandle; same > mParentProxy.reset(const_cast<IAccessibleHolder&>(aParentCOMProxy).Release()); > SetConstructedInParentProcess(); > > for (uint32_t i = 0, l = mDeferredEvents.Length(); i < l; ++i) { > mDeferredEvents[i]->Dispatch(); > } > > mDeferredEvents.Clear(); >+ mEmulatedWindowHandle = aEmulatedWindowHandle; that should probably come before dispatching the events. >+ uintptr_t GetEmulatedWindowHandle() const { return mEmulatedWindowHandle; } HWND >+ uintptr_t mEmulatedWindowHandle; same >@@ -80,16 +80,20 @@ void > a11y::ProxyDestroyed(ProxyAccessible* aProxy) > { > AccessibleWrap* wrapper = > reinterpret_cast<AccessibleWrap*>(aProxy->GetWrapper()); > MOZ_ASSERT(wrapper); > if (!wrapper) > return; > >+ if (aProxy->IsDoc() && nsWinUtils::IsWindowEmulationStarted()) { >+ aProxy->AsDoc()->SetEmulatedWindowHandle(0); >+ } seems pointless, the ctor just set it to 0 didn't it? >+ } else { >+ DocAccessibleParent* docParent = static_cast<DocAccessibleParent*>( >+ ::GetPropW(hWnd, kPropNameDocAccParent)); >+ if (docParent) { >+ auto wrapper = >+ static_cast<DocProxyAccessibleWrap*>(WrapperFor(docParent)); AccessibleWrap* should be fine, drop the cast. r=me with comments fixed.

Hi Marco, so here are the builds from the try run from Comment 35: debug: https://archive.mozilla.org/pub/firefox/try-builds/yura.zenevich@gmail.com-18b487a88f457855c85518e7e5cc6f5eda18940c/try-win64-debug/ opt: https://archive.mozilla.org/pub/firefox/try-builds/yura.zenevich@gmail.com-18b487a88f457855c85518e7e5cc6f5eda18940c/try-win64/ Note that I suspect virtual buffer will not work since Brett mentioned the focus is not set on the emulated window for the tab. If so I think I will have to explicitly do that when new window is created for an active top doc and when active top doc is updated.

I still cannot get the virtual buffer to work at all with this build, but again, I'm using JAWS 17, need to get JAWS 18 to work and re-test later. There may be stuff in there they fixed that will make it work with 18. But this is an important issue because not everybody will be on the latest release of JAWS due to upgrade costs.

Hi Marco, not sure if you will be able to manage to test with JAWS but this try run includes changes that set focus on active emulated window. if you have a moment, could you see if it makes things better? Note WE still isnt recognized. https://treeherder.mozilla.org/#/jobs?repo=try&revision=4f912c672661d3ec5863e57f6820bb77faee2a0e https://archive.mozilla.org/pub/firefox/try-builds/yura.zenevich@gmail.com-4f912c672661d3ec5863e57f6820bb77faee2a0e/

This didn't make any difference to me unfortunately. :-(

Carrying over r+

Marco, would you take a look at this build, i think this one might be it :) https://archive.mozilla.org/pub/firefox/try-builds/yura.zenevich@gmail.com-56606f67ed8f03fe7053326f4186286d43746a99/

(In reply to Yura Zenevich [:yzen] from comment #42) > Marco, would you take a look at this build, i think this one might be it :) > > https://archive.mozilla.org/pub/firefox/try-builds/yura.zenevich@gmail.com- > 56606f67ed8f03fe7053326f4186286d43746a99/ Unfortunately that try build hasn't finished. So I couldn't try anything. :-(

Ah sorry the link didn't paste correctly: https://archive.mozilla.org/pub/firefox/try-builds/yura.zenevich@gmail.com-56606f67ed8f03fe7053326f4186286d43746a99/

Nope, the link is the one I visited, but the sub directory is empty except for a gzipped log file or so. https://archive.mozilla.org/pub/firefox/try-builds/yura.zenevich@gmail.com-56606f67ed8f03fe7053326f4186286d43746a99/try-win64/

Pushed by yura.zenevich@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/dd70938318f8 add support for HWND emulation for content top level docs on Windows. r=tbsaunde

Comment on attachment 8829721 [details] [diff] [review] 1206711 part 4 >- // Accessibles in child processes are said to have the HWND of the window >- // their tab is within. Popups are always in the parent process, and so >- // never proxied, which means this is basically correct. > if (aAccessible->IsProxy()) { > ProxyAccessible* proxy = aAccessible->Proxy(); > if (!proxy) { > return nullptr; > } That we use this code like this is really round about and silly, but we can fix it some other day. > >+ // If window emulation is enabled, retrieve emulated native window from a >+ // containing document proxy. retrieve the emulated window from the containing document ...

Pushed by yura.zenevich@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6589d2f22f6a ensure that emulation is taken into account when resolving HWND for a proxy. r=tbsaunde

Is there more work to be done in this bug, or should it be resolved?

Pushed by yura.zenevich@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/771f1590f03e pass emulated window proxy to doc accessible child to be used as parent IAccessible for top level documents. r=tbsaunde

As per today's meeting, Yura will close this fixed and open compat specific bugs for remaining issues (and add aes+ as appropriate to whiteboard).

Closing and will open bugs that are specific to the issues uncovered.