Bug 115315 - Editing: Cursor not in focus with new Calc/Writer document
Summary: Editing: Cursor not in focus with new Calc/Writer document
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
5.0.0.5 release
Hardware: x86-64 (AMD64) macOS (All)
: high major
Assignee: Not Assigned
URL:
Whiteboard: target:6.1.0 target:6.0.2
Keywords: bibisected, bisected, haveBacktrace, regression
: 115510 115763 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-01-30 20:00 UTC by landi
Modified: 2018-03-10 08:53 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments
Bibisect log (2.90 KB, text/plain)
2018-01-31 20:03 UTC, Telesto
Details
bt with symbols (6.80 KB, text/rtf)
2018-02-10 17:14 UTC, Julien Nabet
Details
Patch (687 bytes, text/plain)
2018-02-12 18:54 UTC, Telesto
Details

Note You need to log in before you can comment on or make changes to this bug.
Description landi 2018-01-30 20:00:26 UTC
When opening a new Calc document, it appears that cell A1 is selected, but apparently the cursor is not actually there or in focus, so you can't immediately begin typing or you can't immediately paste anything. You need to first select A1 (or other cell) before you can type or paste.

Version: 6.1.0.0.alpha0+
Build ID: 85b3c799ede62a3d7ad0493fc80b629214956601
CPU threads: 8; OS: Mac OS X 10.13.3; UI render: default; 
TinderBox: MacOSX-x86_64@49-TDF, Branch:master, Time: 2018-01-29_06:03:03
Locale: en-US (en_US.UTF-8); Calc: group threaded

Regression: This does NOT occur in 5.4.4.2
This has been happening to me with all versions of 6.x

I found some old bugs related to this in Writer, but nothing in Calc, so hopefully I'm not duplicating a bug report.
Comment 1 landi 2018-01-30 20:05:20 UTC
This appears to happen in both Writer and Calc.

However, it only occurs if you use the File menu to create a new document. If you use the StartCenter, then it works as it should.
Comment 2 Alex Thurgood 2018-01-31 08:19:45 UTC
Confirming with

Version: 6.1.0.0.alpha0+
Build ID: 0f28c8612f4269cec95688b53d182c7c0169236d
CPU threads: 4; OS: Mac OS X 10.13.3; UI render: default; 
Locale: fr-FR (fr_FR.UTF-8); Calc: group threaded

Works correctly in 

Version: 5.4.4.2
Build ID: 2524958677847fb3bb44820e40380acbe820f960
Threads CPU : 4; OS : Mac OS X 10.13.3; UI Render : par défaut; 
Locale : fr-FR (fr_FR.UTF-8); Calc: group

==> regression
Comment 3 Telesto 2018-01-31 12:11:51 UTC
Only for the record, not reproducible with Windows
Version: 6.1.0.0.alpha0+
Build ID: ea89dabf8b6363972190a6b50c527c418d51c2c7
CPU threads: 4; OS: Windows 6.3; UI render: default; 
TinderBox: Win-x86@42, Branch:master, Time: 2018-01-27_22:55:15
Locale: nl-NL (nl_NL); Calc: CL
Comment 4 Telesto 2018-01-31 14:06:33 UTC
Repro with
Version: 5.4.0.3
Build ID: 7556cbc6811c9d992f4064ab9287069087d7f62c
CPU threads: 4; OS: Mac OS X 10.12.6; UI render: default; 
Locale: nl-NL (nl_NL.UTF-8); Calc: group

and with
Version: 5.3.7.2
Build ID: 6b8ed514a9f8b44d37a1b96673cbbdd077e24059
CPU Threads: 4; OS Version: Mac OS X 10.12.6; UI Render: default; Layout Engine: new; 
Locale: nl-NL (nl_NL.UTF-8); Calc: group

and with
Version: 5.0.6.3
Build ID: 490fc03b25318460cfc54456516ea2519c11d1aa
Locale: nl-NL (nl_NL.UTF-8)

and with
Version: 5.0.0.5
Build ID: 1b1a90865e348b492231e1c451437d7a15bb262b
Locale: nl-NL (nl_NL.UTF-8)

but not with
Version: 4.4.7.2
Build ID: f3153a8b245191196a4b6b9abd1d0da16eead600
Locale: nl_NL.UTF-8

STR
1. Launch LibreOffice
2. Open a Writer document from Start Center
3. Create a new one (CTRL=N) -> No cursor)
--
4. Open the about screen -> It will open behind document 2 (there might be a relation)
Comment 5 landi 2018-01-31 15:31:49 UTC
Mac OS has an OS-level feature where you can press Command-` (the punctuation mark above the tab key, also called accent or acute) to switch focus to the next window. This does not work in 6.x. Calc, though it does work in 6.x Writer. Since this is a focus-related issue, could this be related?
Comment 6 Telesto 2018-01-31 20:03:40 UTC
Created attachment 139475 [details]
Bibisect log

Bisected to
author	Michael Meeks <michael.meeks@collabora.com>	2015-06-05 16:37:49 +0100
committer	Michael Meeks <michael.meeks@collabora.com>	2015-06-05 19:45:12 +0000
commit 7aae8772aa18744cb1bbd8348272be99cc882c47 (patch)
tree c4824605af0def9fe11cb1c302f9b0e18993c06e
parent b9f95769495e8d9885b64f1d68118336a4fc8d38 (diff)
Clear VclPtr instance reference on removed UserEvents.
Also extend VclPtr lifecycle test.
Comment 7 Telesto 2018-01-31 20:23:03 UTC
Adding CC to Michael Meeks
Comment 8 Telesto 2018-02-01 10:46:54 UTC
This - a partial revert - would fix the issue:

diff --git a/vcl/source/window/window.cxx b/vcl/source/window/window.cxx
index aedd405f1c04..438b6876fdf5 100644
--- a/vcl/source/window/window.cxx
+++ b/vcl/source/window/window.cxx
@@ -488,7 +488,7 @@ void Window::dispose()
     if ( pSVData->maWinData.mpLastDeacWin == this )
         pSVData->maWinData.mpLastDeacWin = nullptr;
 
-    if ( mpWindowImpl->mpFrameData )
+    if ( mpWindowImpl->mbFrame && mpWindowImpl->mpFrameData )
     {
         if ( mpWindowImpl->mpFrameData->mnFocusId )
             Application::RemoveUserEvent( mpWindowImpl->mpFrameData->mnFocusId );
Comment 9 Telesto 2018-02-01 13:28:56 UTC
Worse case scenario is a crash, in the current situation:
1. Launch LibreOffice
2. Open a Writer document from Start Center
3. Create a second one (CTRL+N) -> No cursor)
4. Open LibreOffice -> About
5. Close document 2
6. Close the about dialog -> Crash
Comment 10 landi 2018-02-01 14:33:51 UTC
(In reply to Telesto from comment #9)

I can confirm this crash, using

Version: 6.1.0.0.alpha0+
Build ID: 85b3c799ede62a3d7ad0493fc80b629214956601
CPU threads: 8; OS: Mac OS X 10.13.3; UI render: default; 
TinderBox: MacOSX-x86_64@49-TDF, Branch:master, Time: 2018-01-29_06:03:03
Locale: en-US (en_US.UTF-8); Calc: group threaded
Comment 11 Michael Meeks 2018-02-10 15:10:01 UTC
The suggested revert is rather unclear to me =) but if it works I'm not against it particularly; except the mbFrame thing is really not so clear to me as to its purpose as distinct from having valid mpFrameData:

IMPL_LINK_NOARG(vcl::Window, ImplAsyncFocusHdl, void*, void)
{
    ImplGetWindowImpl()->mpFrameData->mnFocusId = nullptr;

is what we remove - if this gets called on a disposed window - with no ImplGetWindowImpl()->mpFrameData - and de-references that, we will crash hard.

It is hard to think how not calling ImplAsyncFocusHdl on a disposed window is going to make life better generally for anything.

Clearly - there is some twisty horror in here; I suspect that perhaps having a FocusToplevel - that does just this piece:

            if ( !bHandled )
            {
                ImplSVData* pSVData = ImplGetSVData();
                vcl::Window*     pTopLevelWindow = ImplGetWindowImpl()->mpFrameData->mpFocusWin->ImplGetFirstOverlapWindow();

                if ((!pTopLevelWindow->IsInputEnabled() || pTopLevelWindow->IsInModalMode()) && !pSVData->maWinData.mpExecuteDialogs.empty())
                    pSVData->maWinData.mpExecuteDialogs.back()->ToTop(ToTopFlags::RestoreWhenMin | ToTopFlags::GrabFocusOnly);
                else
                    pTopLevelWindow->GrabFocus();
            }

Might be better - to call in the case that we're being disposed with focus; but ... the whole behavior when shutting down is really not hyper-clear to me.

Ideally I guess the new window would call 'GrabFocus()' in some clear place on as it starts; hmmm.

So - more thinking required than I have time for unfortunately just reviewing the patch; sorry !
Comment 12 Julien Nabet 2018-02-10 16:54:52 UTC
(In reply to Telesto from comment #9)
> Worse case scenario is a crash, in the current situation:
> 1. Launch LibreOffice
> 2. Open a Writer document from Start Center
> 3. Create a second one (CTRL+N) -> No cursor)
> 4. Open LibreOffice -> About
> 5. Close document 2
> 6. Close the about dialog -> Crash

On MacOS 10.13.3 with master sources updated some days ago, I don't reproduce the initial pb with Calc but reproduce the crash described here.
The only thing is after 4., I don't see About window.
It's only when I close doc 2 (5.) that I can see the About window and then it crashes after 6.
Comment 13 Julien Nabet 2018-02-10 17:14:43 UTC
Created attachment 139762 [details]
bt with symbols

I attached bt from crash described by Telesto.
There's also some extra info from lldb session.
Comment 14 Telesto 2018-02-10 17:18:57 UTC
(In reply to Julien Nabet from comment #13)

I bibisected this with the following steps (comment 4, at the bottem)
1. Launch LibreOffice
2. Open a Writer document from Start Center
3. Create a new document(CTRL+N). No cursor in the new document
Comment 15 Julien Nabet 2018-02-10 17:26:31 UTC
(In reply to Telesto from comment #9)
> Worse case scenario is a crash, in the current situation:
> 1. Launch LibreOffice
> 2. Open a Writer document from Start Center
> 3. Create a second one (CTRL+N) -> No cursor)
> 4. Open LibreOffice -> About
> 5. Close document 2
> 6. Close the about dialog -> Crash

With the patch proposed on comment 8, I don't have a crash.
1., 2., 3., 4. are ok. (so this time About dialog displays)
But when trying 5., it does nothing.
6. no crash
and I can see there are still 2 Writer docs.
Comment 16 Julien Nabet 2018-02-10 17:29:58 UTC
BTW, I confirm that without the patch, when opening a second Writer document, I got no cursor.

(notice: I didn't remind it after my first comment but all my following comments concern MacOS 10.13.3)
Comment 17 Maxim Monastirsky 2018-02-10 18:46:32 UTC
(In reply to Michael Meeks from comment #11)
> except the mbFrame thing is really not so clear to
> me as to its purpose as distinct from having valid mpFrameData
mbFrame means _this_ vcl::Window is a frame, while a valid mpFrameData only means that the current vcl::Window presents in the hierarchy of some other frame window, typically some control inside that window. This can be clearly seen at the top of Window::ImplInsertWindow:

if ( pParent && !mpWindowImpl->mbFrame )
{
    // search frame window and set window frame data
    vcl::Window* pFrameParent = pParent->mpWindowImpl->mpFrameWindow;
    mpWindowImpl->mpFrameData     = pFrameParent->mpWindowImpl->mpFrameData;

so mpFrameData is just a pointer to the mpFrameData of the parent frame window. 

So:

> It is hard to think how not calling ImplAsyncFocusHdl on a disposed window
> is going to make life better generally for anything.
ImplAsyncFocusHdl isn't necessarily called on the window we're disposing. Consider the scenario: A child control is disposed, and because of the missing mpWindowImpl->mbFrame check it clears the focus event of the (not disposed!) top level frame...
Comment 18 Michael Meeks 2018-02-10 18:50:48 UTC
Ah - fair cop, Maxim convinced me =) can you push the partial revert ? would be good to have it in older versions too of course.

Thanks !
Comment 19 Alex Thurgood 2018-02-12 12:01:40 UTC
*** Bug 115510 has been marked as a duplicate of this bug. ***
Comment 20 Telesto 2018-02-12 18:54:36 UTC
Created attachment 139837 [details]
Patch

A different approach; Mac only.
Comment 21 Maxim Monastirsky 2018-02-12 20:55:49 UTC
(In reply to Telesto from comment #20)
> Created attachment 139837 [details]
> Patch
> 
> A different approach; Mac only.
Telesto: I don't think that a Mac specific change is the way to go, as I can reproduce the bug under Fedora 27 too, and the partial revert of commit 8 does fix the problem there too. So as Michael asked, please submit the partial revert to gerrit. Thanks.
Comment 22 Alex Thurgood 2018-02-16 07:58:17 UTC
*** Bug 115763 has been marked as a duplicate of this bug. ***
Comment 23 Maxim Monastirsky 2018-02-16 12:32:03 UTC
To move things forward, I submitted the patch to gerrit myself:

https://gerrit.libreoffice.org/49855
Comment 24 Michael Meeks 2018-02-16 17:28:29 UTC
Thanks Maxim ! - pushed to master and cherry-picked to 6-0 =)
Comment 25 Commit Notification 2018-02-16 17:28:48 UTC
Telesto committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=11fab5aeaaa72012c63b2c812656a932ef0debf9

tdf#115315 Cursor not in focus with new document

It will be available in 6.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 26 landi 2018-02-21 20:09:25 UTC
I'm unable to find a recent master build with a new build date. Am I looking in the wrong place https://dev-builds.libreoffice.org/daily/master/MacOSX-x86_64@49-TDF/current/
Comment 27 Commit Notification 2018-02-21 20:35:07 UTC
Telesto committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=fcbcbe607ba3c693c79b1e1c390fab3b4643e3d9&h=libreoffice-6-0

tdf#115315 Cursor not in focus with new document

It will be available in 6.0.2.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 28 landi 2018-02-26 15:49:23 UTC
Is there a patch available for testing anywhere? I seem to be unable to find one.
Comment 29 Julien Nabet 2018-02-26 15:54:53 UTC
(In reply to landi from comment #28)
> Is there a patch available for testing anywhere? I seem to be unable to find
> one.

If you build from sources, you can update your local repository or use the Telesto's patch here:
https://cgit.freedesktop.org/libreoffice/core/commit/?id=fcbcbe607ba3c693c79b1e1c390fab3b4643e3d9&h=libreoffice-6-0

About MacOs daily build, I searched here https://dev-builds.libreoffice.org/daily/ on 6.0 and master branch, I found nothing.
Comment 30 Telesto 2018-02-26 16:55:55 UTC
(In reply to landi from comment #28)
> Is there a patch available for testing anywhere? I seem to be unable to find
> one.

https://dev-builds.libreoffice.org/pre-releases/mac/x86_64/LibreOffice_6.0.2.1_MacOS_x86-64.dmg
Comment 31 landi 2018-02-26 23:44:24 UTC
I've tested in Writer and Calc, and it seems to be working. I created new documents using the File menu, and using command-N, and using the Start Center. It worked with all methods on multiple documents.
Comment 32 Cyrille 2018-03-10 08:53:51 UTC
the bug 115315 (duplicate of bug 115510) is completely resolved with version 6.0.2.1