Bug 155393 - Crash in SfxShell::GetViewShell()
Summary: Crash in SfxShell::GetViewShell()
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
7.2.0.0 alpha1+
Hardware: All All
: medium normal
Assignee: Jim Raykowski
URL:
Whiteboard: target:7.6.0 target:7.5.5 inReleaseNo...
Keywords: bibisected, bisected, haveBacktrace, regression
Depends on:
Blocks: Navigator Crash
  Show dependency treegraph
 
Reported: 2023-05-18 18:56 UTC by Xisco Faulí
Modified: 2024-04-25 04:04 UTC (History)
6 users (show)

See Also:
Crash report or crash signature: ["SfxShell::GetViewShell()","sd::DrawViewShell::ExecNavigatorWin(SfxRequest&)"]


Attachments
screencast (3.84 MB, video/mp4)
2023-05-18 18:56 UTC, Xisco Faulí
Details
bt with debug symbols (5.60 KB, text/plain)
2023-05-18 21:16 UTC, Julien Nabet
Details
demo: grey out navigator when in notes view (329.21 KB, video/x-matroska)
2023-05-21 01:06 UTC, Jim Raykowski
Details
Notes view navigator demo (1.12 MB, video/x-matroska)
2023-05-21 03:31 UTC, Jim Raykowski
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Xisco Faulí 2023-05-18 18:56:39 UTC
Created attachment 187382 [details]
screencast

Steps to reproduce:
1. Open Impress
2. Open the navigator -> Slide 1 is selected
3. View - Notes
4. Once Slide 1 gets deselected in the navigator click many times on Shape 1

-> Crash. See screencast

Reproduced in

Version: 7.6.0.0.alpha1+ (X86_64) / LibreOffice Community
Build ID: 343f1910d40f6d89658cf6e30b9b8f4af51deb43
CPU threads: 8; OS: Linux 5.10; UI render: default; VCL: x11
Locale: es-ES (es_ES.UTF-8); UI: en-US
Calc: threaded
Comment 1 Xisco Faulí 2023-05-18 18:57:49 UTC
@Caolán, I thought you might be interested in this issue
Comment 2 Julien Nabet 2023-05-18 21:16:14 UTC
Created attachment 187384 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated today, I could reproduce this.
Comment 3 Stéphane Guillou (stragu) 2023-05-19 12:26:51 UTC
Also in 7.5:

Version: 7.5.3.2 (X86_64) / LibreOffice Community
Build ID: 9f56dff12ba03b9acd7730a5a481eea045e468f3
CPU threads: 8; OS: Linux 5.15; UI render: default; VCL: gtk3
Locale: en-AU (en_AU.UTF-8); UI: en-US
Calc: threaded

And in 7.2 but not 7.1.

Crash reports:
- 7.5: https://crashreport.libreoffice.org/stats/crash_details/3a9272a7-f601-452e-98d2-677e5b61c9ce
- 7.2: https://crashreport.libreoffice.org/stats/crash_details/a750de4a-452e-4f72-a75e-571ed70dfacb

Bibisected with linux-64-7.2 repo to first bad commit 3ff934f2241a47628ac920c616648403dfc98273 which points to core commit:

commit f0878173e1963cf8db5f60ced6d19da24e18bc41
author	Jim Raykowski <raykowj@gmail.com>	Tue Dec 01 00:25:52 2020 -0900
committer	Jim Raykowski <raykowj@gmail.com>	Thu Dec 03 21:38:48 2020 +0100
tdf#34828 sd navigator: make unnamed shape select select shape object
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106958

Jim, can you please have a look?
Comment 4 Jim Raykowski 2023-05-21 01:06:10 UTC
Created attachment 187415 [details]
demo: grey out navigator when in notes view

For me, getting this to crash using x11 or gtk3 isn't easy. With qt5 the crash happens as soon as an object in the Navigator is clicked on. Perhaps an issue related to when state changes get fired. 

The small patch offered here makes the Navigator grey out when in Notes view. With vintage computers it is still possible to repro the crash by being quick to click on an object in the tree before the timer is invoked that updates the navigator controller item states:

https://gerrit.libreoffice.org/c/core/+/152027

With some amount of enhancement hacking it looks to be possible to show in the Navigator, slides and page objects of Notes when in the Notes view, and normal slides and objects when the view is in Normal view.
Comment 5 Jim Raykowski 2023-05-21 03:31:49 UTC
Created attachment 187416 [details]
Notes view navigator demo

Here is a demo of slides and page objects in Notes view being shown in the Navigator.
Comment 6 Jim Raykowski 2023-05-21 23:49:30 UTC
Here is a link to a patch that allows navigation of the Notes view:
https://gerrit.libreoffice.org/c/core/+/152075
Comment 7 Stéphane Guillou (stragu) 2023-05-25 12:14:55 UTC
(In reply to Jim Raykowski from comment #5)
> Created attachment 187416 [details]
> Notes view navigator demo
> 
> Here is a demo of slides and page objects in Notes view being shown in the
> Navigator.

This looks great, thanks Jim! Making the navigator useful to the Notes view is a way better solution, thanks for going in that direction.

I couldn't find a corresponding enhancement request on Bugzilla, but it looks to me like a non-controversial, welcome addition.

Copying the UX team in in any case.
UX team: Jim was just going to fix a crash (gerrit#152027) but also explored making the Navigator functional for Notes view as an alternative (video and gerrit#152075). Thoughts?
Comment 8 Heiko Tietze 2023-05-25 12:39:15 UTC
Having the Navigator functional for Notes is a big win. Disabling it would be odd as you cannot jump to a certain slide. Anything in particular that needs UX review?
Comment 9 Stéphane Guillou (stragu) 2023-05-25 13:07:35 UTC
(In reply to Heiko Tietze from comment #8)
> Having the Navigator functional for Notes is a big win. Disabling it would
> be odd as you cannot jump to a certain slide. Anything in particular that
> needs UX review?

Nothing in particular, just wanted to make sure others were notified and were able to give feedback as there is no enhancement request linked to this feature. (And possibly get a +1 on gerrit!)
Comment 10 Heiko Tietze 2023-05-30 09:53:10 UTC
(In reply to Stéphane Guillou (stragu) from comment #9)
> just wanted to make sure others were notified...
LGTM
> (And possibly get a +1 on gerrit!)
done
Comment 11 Commit Notification 2023-05-30 19:07:14 UTC
Jim Raykowski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/60e32969a98cad348cf8e55e8f93abc3d6e9c70c

tdf#155393 SdNavigator: Enhancement to navigate in Notes view

It will be available in 7.6.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.
Comment 12 Jim Raykowski 2023-05-30 19:42:37 UTC
Marking this a resolved fixed. Please let me know if additional work is needed.
Comment 13 Commit Notification 2023-06-02 10:40:16 UTC
Jim Raykowski committed a patch related to this issue.
It has been pushed to "libreoffice-7-5":

https://git.libreoffice.org/core/commit/b23077e1912c41905b44c15c914cb6db889cd22b

tdf#155393 SdNavigator: Enhancement to navigate in Notes view

It will be available in 7.5.5.

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.
Comment 14 Stéphane Guillou (stragu) 2023-11-15 21:57:04 UTC
mentioned in release notes: https://wiki.documentfoundation.org/index.php?title=ReleaseNotes%2F7.5&type=revision&diff=706180&oldid=682511
Comment 15 Stéphane Guillou (stragu) 2024-04-25 04:04:05 UTC
Crash fix verified in:

Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 695e8742da850bbb15c2e6d2b5d4c99a0daf4925
CPU threads: 8; OS: Linux 6.5; UI render: default; VCL: gtk3
Locale: en-AU (en_AU.UTF-8); UI: en-US
Calc: CL threaded