Merge all vbufBackend dlls into nvdaHelperRemote.dll#8866
Conversation
| return NULL; | ||
| } | ||
|
|
||
| auto i=VBufBackendFactoryMap.find(backendName); |
There was a problem hiding this comment.
If backendName is nullptr then I don't think it will convert to a wstring
| auto i=VBufBackendFactoryMap.find(backendName); | |
| if(!backendName){ | |
| LOG_ERROR(L"BackendName is nullptr"); | |
| return nullptr; | |
| } | |
| auto i=VBufBackendFactoryMap.find(backendName); |
| backend->initialize(); | ||
| // Stop nvdaHelperRemote from being unloaded while a backend exists. | ||
| HINSTANCE tempHandle=nullptr; | ||
| if(!GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS,(LPCTSTR)dllHandle,&tempHandle)) { |
There was a problem hiding this comment.
Could we replace the c-style cast in here (LPCTSTR)dllHandle
| } | ||
| return TRUE; | ||
| } | ||
| UINT WM_HTML_GETOBJECT=RegisterWindowMessage(L"WM_HTML_GETOBJECT"); |
There was a problem hiding this comment.
This potentially subtly changes the order/timing of when this happens. This might not be a problem, but it's worth being aware of. If possible, I would prefer to see an explicit initialisation function.
|
@feerrenrut I have addressed your review comments. |
This reverts commit 24f5123.
…ted in braille and speech" (#8899) * Revert "Revert "Don't announce 'selected' when the focus moves in Google sheets if the focused cell is the only cell selected (#8879)" (#8893)" This reverts commit b4e9e83. * Revert "Don't announce 'selected' when the focus moves in Google sheets if the focused cell is the only cell selected (#8879)" This reverts commit 51f5b2f. * Revert "Merge all vbufBackend dlls into nvdaHelperRemote.dll (#8866)" This reverts commit 24f5123. * Revert "Fix handling of table cells without a containing table in browse mode. (#8887)" This reverts commit 5fe34c5. * Revert "Ensure that labels explicitly set on divs and spans are reported in braille and speech (#8886)" This reverts commit fd24d81.
|
Apologies for disturbing if this is irrelevant: Beware two branches were created with similar names but different case: mergedVbufBackendLibs (lower b) and mergedVBufBackendLibs (upper B), the former being ahead of the latter by 5 commits. |
|
We can remove the mergedVBufBackendLibs branch, but I'm not going to do that without @michaelDCurran's consent. |
|
Feel free to remove it. If you don't have permission, let me know and I
can do it.
|
Link to issue number:
Closes #7641
Summary of the issue:
After updating NVDA while Firefox or Chrome web browsers are open, NVDA's virtualBuffers in these browsers fail to reflect further updates to web pages, and in some cases the browser crashes.
This is caused by:
Description of how this pull request fixes the issue:
This PR removes the possibility of loading the wrong nvdaHelperRemote.dll by moving all the virtualBuffer code into nvdaHelperRemote.dll itself, which removes the need for the vbufBackend dlls all together.
The vbufBackend dls were origianlly separate as there was some hope that perhaps one day we might allow custom backends. However, in truth this was never fully possible due to other limitations in NVDA. Another reason was that vbufBackend dlls may link against some specific heavy-weight libraries, which in many cases would not be required in all apps except the ones that actually required that specifica virtualBuffer. In truth though, none of the vbufBackend dlls link against anything else other than what nvdaHelperRemote already links against.
A part from solving the dynamic changes dying, and the crashes, a further advantage is that NvDA's size shrinks significantly as we were carrying around about 12 vbufBackend dlls, all containing their own static copy of the CRT. Now, the virtualBuffers all share the one crt in nvdaHelperRemote.dll.
Testing performed:
Ran NVDA. Opened documents that require virtualBuffers in Firefox, Chrome, Internet Explorer, and webKit (iTunes). AdobeAcrobat, AdobeFlash and LotusNotes are yet to be tested. However, the only way these could fail is if the name if the virtualBuffer has been typed incorrectly in the source code. The rest of the code stays the same.
Known issues with pull request:
None.
Change log entry:
Bug fixes:
After updating NVDA, NvDA no longer fails to notice updates to document in Firefox and these browsers should no longer crash after the NVDA update. (#7641)