Fix crashes in 64-bit builds of Notepad++ 8.3 and later

[lukaszgo1]

Link to issue number:

Fixes #13311

Summary of the issue:

NVDA causes a crash when interacting with 64-bit builds of Notepad+++ 8.3. This happens because Notepad++ developers decided to deviate from the official Scintilla interface (they character points are represented by longlong rather than long in 64-bit builds). Since this allowed them to introduce the support for much bigger files this change is going to stay and NVDA has to adapt (see explanations in notepad-plus-plus/notepad-plus-plus#11133).

Description of how this pull request fixes the issue:

For 64-bit builds of Notepad++ 8.3 and above longlong is used for representing characters points.

Testing strategy:

Tested that:

• Notepad++ 8.3 x64 no longer crashes and text can be read normally.
• 32-bit build of the same version of NPP works as well as it used to before this change.
• Older version of Notepad++ x64 works normally and long is used for characters points there.
• Different editor (teXnicCenter) using Scintilla is not negatively affected.
• The newly introduced appmodule can be imported by using from nvdaBuiltin.appModules import notepadPlusPlus in an add-on.

Known issues with pull request:

To make this work for add-ons wanting to extend the built-in appModule the 'real' module is named notepadPlusPlus and then it is mapped to the notepad++ via an dummy alias appModule. The alias module would be removed as soon as #13366 is merged.

Change log entries:

Bug fixes

• NVDA no longer causes 64-bit versions of Notepad++ 8.3 and above to crash.

For Developers

• NVDAObjects.window.scintilla.CharacterRangeStruct becomes NVDAObjects.window.scintilla.Scintilla.CharacterRangeStruct

Code Review Checklist:

• Pull Request description:
  • description is up to date
  • change log entries
• Testing:
  • Unit tests
  • System (end to end) tests
  • Manual testing
• API is compatible with existing add-ons.
• Documentation:
  • User Documentation
  • Developer / Technical Documentation
  • Context sensitive help for GUI changes
• UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers
  • Localization in other languages / culture than English

[lukaszgo1]

@seanbudd It would be awesome to include this in 2022.1 as Notepad++ is one of the most accessible code editors out there, these crashes are pretty ugly and historically NVDA always offered the best support for it.

[aaclause]

Thanks @lukaszgo1! Looks good to me except with Notepad++ add-on enabled. Any idea what should be changed in this add-on to make it work? (no traceback, the app crashes like without this fix...)

[lukaszgo1]

@aaclause Thanks for your comment. I intent to submit a pull request to the Notepad++ add-on to fix this - unfortunately this cannot be done at present. I've updated the top comment to explain why this module cannot be extended in an add-on, and will pick this up again as soon as #13366 is merged.

[CyrilleB79]

Known issues with pull request:

* This currently breaks if someone is using an add-on for Notepad++. A normal way of extending a built-in appModule in an add-on i.e. importing it from `nvdaBuiltin` cannot work here since the module contains a "_" in its name and Python's import system cannot deal with this character as part of import statement. I've opened [Improve flexibility  of appModules discovery by allowing to map arbitrary appModule to a given executable. #13366](https://github.com/nvaccess/nvda/pull/13366) and as soon as it gets merged will modify this PR so that the appModule is named in a way which does not cause troubles for Python's import.

Sorry if I have missed something, but some points need to be clarified for me:

1. You write

the module contains a "_" in its name
Don't you mean rather a "+" (notepad++)?

2. If you cannot write:
   from notepad++ import *
   could you use importlib.import_module instead?
   E.g.
   npp = importlib.import_module('notepad++')
   And then update globals() with the names in npp.
   A more precise description of this process is here.

[lukaszgo1]

Thanks @CyrilleB79 I've updated the top comment to explain what alternatives to merging #13366 are available. Having said that this is not the first time when current system of mapping appModules to binaries proves not to be flexible enough so my opinion is that we should go for #13366 if at all possible.

[CyrilleB79]

Thanks @CyrilleB79 I've updated the top comment to explain what alternatives to merging #13366 are available. Having said that this is not the first time when current system of mapping appModules to binaries proves not to be flexible enough so my opinion is that we should go for #13366 if at all possible.

Thanks for this clarification.
Even if there is already a possibility to import notepad++.py, I agree with you that providing a more straight-forward way to do it in NVDA would be a plus for everybody.

[seanbudd]

I would suggest the following option if #13366 isn't mergeable

NVDA should bundle an alias appModule (named for example notepadPlusPlus) which would just load everything from the notepad++ module so that this dummy module can be imported from nvdaBuiltin

[seanbudd]

I would suggest the following option if #13366 isn't mergeable

NVDA should bundle an alias appModule (named for example notepadPlusPlus) which would just load everything from the notepad++ module so that this dummy module can be imported from nvdaBuiltin

In fact, can you go ahead and do this in this PR? That way this can be tested on alpha while #13366 is resolved.
Then we can follow up and remove this alias #13366

[lukaszgo1]

@seanbudd This is ready for review. I've inverted the approach i.e. the real module is named notepadPlusPlus so that add-ons would not need to be modified after #13366 is merged, and notepad++.py just exposes its content.

[seanbudd]

Looks great, thanks @lukaszgo1

[LeonarddeR]

I'm experiencing crashes with 8.2.1 now.