Created attachment 197313 [details] Backtrace of the assert Steps to reproduce: 1) start NVDA screen reader 2) start LO Writer 3) "Tools" > "Macros" > "Edit Macros" 4) Press Enter key 5) if nothing happens yet, repeat step 4 and/or keep the Enter key pressed Result: In a debug build, an assert is triggered, s. attached backtrace. Another potential way to misbehave seen is the one Mike describes in bug 160982 comment #8: > (...) with some assistive technology > active, I see in Basic IDE, that pressing and holding Enter to add multiple > new paragraphs, the current line with the cursor goes beyond the window > (previously, the cursor was always at the bottom, and the window expectedly > scrolled to keep it visible). As mentioned in bug 160982 comment #9, I think the underlying issue is preexisting, but in some setups (where an AT is active that doesn't set the SPI_GETSCREENREADER flag), this issue is triggered now where it wasn't without the fix for tdf#160982 in place. (It can also be seen in older LO with NVDA, which does set the flag). I don't see this happening with gtk3 on Linux, also not when Orca is running. Version: 25.2.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: 0e05add86cfb996fe6a11c088c36b3fa010e2445 CPU threads: 12; OS: Windows 10 X86_64 (10.0 build 19045); UI render: default; VCL: win Locale: en-GB (en_GB); UI: en-US Calc: threaded
(In reply to Michael Weghorn from comment #0) > Steps to reproduce: > > 1) start NVDA screen reader > 2) start LO Writer > 3) "Tools" > "Macros" > "Edit Macros" > 4) Press Enter key > 5) if nothing happens yet, repeat step 4 and/or keep the Enter key pressed This happens when the cursor is at the end of the document, i.e. between step 3) and 4), do 3a) Move cursor to the end of the text window
Note that there are two problems causing the crash. 1. In Document::handleParagraphNotifications, when SfxHintId::TextParaInserted is called (i.e., a new para is inserted), the call to m_xParagraphs->insert uses the height obtained from a call to m_rEngine.GetTextHeight for that paragraph. But that call would eventually cause a broadcast of SfxHintId::TextFormatPara - which means, that at this point, handleParagraphNotifications will be re-entered, and the handler (below) will try to access a yet-missing element in m_xParagraphs. This likely should simply be refactored to (first) insert the new element with 0 height, and (second) set its height to the value obtained from m_rEngine.GetTextHeight. 2. The re-entry to handleParagraphNotifications will also use the iterators 'm_aVisibleBegin/End' and 'm_aFocused'; but they are invalidated by the call to m_xParagraphs->insert, so they need to be reset to valid values *before* calling m_rEngine.GetTextHeight. In fact, storing iterators in the Document object is bad. It would make sense to drop them, and only store numerical offsets.
(In reply to Mike Kaganski from comment #2) > 2. The re-entry to handleParagraphNotifications will also use the iterators Should be fixed with https://gerrit.libreoffice.org/c/core/+/175852. Only #1 stays.
(In reply to Mike Kaganski from comment #2) > Note that there are two problems causing the crash. > > 1. In Document::handleParagraphNotifications, when > SfxHintId::TextParaInserted is called (i.e., a new para is inserted), the > call to m_xParagraphs->insert uses the height obtained from a call to > m_rEngine.GetTextHeight for that paragraph. But that call would eventually > cause a broadcast of SfxHintId::TextFormatPara - which means, that at this > point, handleParagraphNotifications will be re-entered, and the handler > (below) will try to access a yet-missing element in m_xParagraphs. > > This likely should simply be refactored to (first) insert the new element > with 0 height, and (second) set its height to the value obtained from > m_rEngine.GetTextHeight. Another alternative might possibly be to call TextEngine::GetTextHeight on demand instead of doing so in Document::handleParagraphNotifications and storing the value. (In reply to Mike Kaganski from comment #3) > (In reply to Mike Kaganski from comment #2) > > 2. The re-entry to handleParagraphNotifications will also use the iterators > > Should be fixed with https://gerrit.libreoffice.org/c/core/+/175852. Only #1 > stays. Unassigning for now, as I see you also started working on this issue already. Feel free to self-assign :)
(In reply to Michael Weghorn from comment #4) > Another alternative might possibly be to call TextEngine::GetTextHeight on > demand instead of doing so in Document::handleParagraphNotifications and > storing the value. I am not sure what you mean. Especially, I have no idea about the point at which the demand may happen, and if it's OK to rely on that, and if it's OK to have some wrong intermediate height in the paragraph list after the handler finished. > Unassigning for now, as I see you also started working on this issue > already. Feel free to self-assign :) I will fix the crash, but please take a look at it after that - exactly because you know the workflow, and can see what could be improved. Additionally, the "non-scrolling to cursor" looks a separate issue, unfixed by the crash fix...
(In reply to Mike Kaganski from comment #5) > I am not sure what you mean. Especially, I have no idea about the point at > which the demand may happen, and if it's OK to rely on that, and if it's OK > to have some wrong intermediate height in the paragraph list after the > handler finished. As `ParagraphInfo` is just a helper class, my idea was to possibly drop ParagraphInfo::m_nHeight, and replace all current uses of ParagraphInfo::getHeight by calling TextEngine::GetTextHeight with the corresponding paragraph number directly. > I will fix the crash, but please take a look at it after that - exactly > because you know the workflow, and can see what could be improved. Thanks! Sure, I'm happy to test the use cases I'm aware of. (Checking that Accerciser highlights correct area for paragraph, and "select object under mouse" feature works.)
https://gerrit.libreoffice.org/c/core/+/175879
The crash should be fixed now.
Mike Kaganski committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/9a07b6c34c12d998b0428a5f70a4833bade3ef1f tdf#163697: avoid premature handling of following notifications It will be available in 25.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
(In reply to Mike Kaganski from comment #8) > The crash should be fixed now. Indeed, thanks! Identifying the object under the mouse generally seems to work fine on Windows as well, as printing information of a paragraph in the macro editor gives the correct information for a previously hovered-over paragraph, e.g.: >>> mouse.role <Role.PARAGRAPH: 47> >>> mouse.IAccessibleTextObject.text(0, -1) 'hello world' and the Accerciser cases with gtk3 on Linux still work as expected, as does some further testing there. (Didn't see the recursion problem with gtk3 either, however.)