Bug 114839 - During execution of the Document Recovery dialog, Dialog::Execute() keeps looping even if nothing happens
Summary: During execution of the Document Recovery dialog, Dialog::Execute() keeps lo...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
6.1.0.0.alpha0+
Hardware: x86-64 (AMD64) macOS (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.3.0
Keywords:
: 104030 (view as bug list)
Depends on:
Blocks: Document-Recovery
  Show dependency treegraph
 
Reported: 2018-01-04 20:50 UTC by Tor Lillqvist
Modified: 2018-11-27 10:59 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:
Regression By:


Attachments
Possible solution (2.44 KB, text/plain)
2018-02-01 13:23 UTC, Telesto
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tor Lillqvist 2018-01-04 20:50:43 UTC
Description:
If LibreOffice shows the Document Recovery dialog when it starts, while the dialog is shown the code in Dialog::Execute() keeps looping in the while ( !xWindow->IsDisposed() && mbInExecute ) loop, calling Application::Yield() again and again endlessly, even if the user doesn't touch anything, move the mouse etc.

Steps to Reproduce:
Add some debugging output into Dialog::Execute() in vcl/source/window/dialog.cxx and rebuild vcl:

    while ( !xWindow->IsDisposed() && mbInExecute )
        { SAL_DEBUG("Dialog::Execute: Calling Application::Yield()");
        Application::Yield();
        }

Run instdir/LibreOfficeDev.app/Contents/MacOS/soffice. (The started LibreOffice will not get focus, that is another issue, and not serious, as end-users of course aren't supposed to start LibreOffice from the Terminal by explicitly running the soffice binary.) Hit Cmd-N to open a fresh Writer document, type a few words, switch back to the Terminal and kill soffice with Control-C.

Run soffice again. You will see an endless loop of debug:1991:499247: Dialog::Execute: Calling Application::Yield() even before you switch focus to the recovery window, and while it has focus. (If you switch focus away, it will no longer loop.)


Actual Results:  
.

Expected Results:
Once it reaches a steady state, it should not keep calling Application::Yield() all the time while nothing happens.


Reproducible: Always


User Profile Reset: No



Additional Info:


User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_2) AppleWebKit/604.4.7 (KHTML, like Gecko) Version/11.0.2 Safari/604.4.7
Comment 1 Telesto 2018-01-05 08:18:19 UTC
Any relation with bug 104030?
Comment 2 Tor Lillqvist 2018-01-05 08:25:34 UTC
Possibly.
Comment 3 Telesto 2018-02-01 13:23:38 UTC
Created attachment 139495 [details]
Possible solution

I might found the cause (or a clue). The problem will disappear when commenting out the following lines.

diff --git a/vcl/osx/salnativewidgets.cxx b/vcl/osx/salnativewidgets.cxx
index d311526c0168..32f940516bd5 100644
--- a/vcl/osx/salnativewidgets.cxx
+++ b/vcl/osx/salnativewidgets.cxx
@@ -64,10 +64,10 @@ public:
     {
         if( AquaSalFrame::isAlive( mpFrame ) && mpFrame->mbShown )
         {
-            mpFrame->maBlinkers.remove( this );
-            mpFrame->SendPaintEvent( &maInvalidateRect );
+     //       mpFrame->maBlinkers.remove( this );
+      //      mpFrame->SendPaintEvent( &maInvalidateRect );
         }
-        delete this;
+      //  delete this;
     }
 };

More in general, what does Aquablink do? Creating a glow button or something like that? I didn't notice any difference without it (patch attached). Note: I didn't delete the reference in schedule
Comment 4 Tor Lillqvist 2018-02-01 13:37:22 UTC
Apparently, back in the days, the default button of dialogs in Mac OS X did "pulsate" or "blink". See https://arstechnica.com/gadgets/2000/02/mac-os-x-dp3/4/ .

Apparently our code for that was introduced in 2007, in 58647e72cf6e84a8dd0645fa6c1f27704d7ce9c0 , INTEGRATION: CWS aquavcl03 (1.3.2); FILE MERGED . One line of that commit message says "2007/09/28 15:10:15 pl 1.3.2.15: blinking default button". In https://wiki.openoffice.org/wiki/Log_Mac_Meeting_October_4th_2007 one sees "even the buttons are animated now ;-) Thanks PL!" 

But as these things tend to go, I wouldn't be surprised if around the same time that OOo started providing support for pulsating buttons, Mac OS X stopped doing that;) So yeah, most likely that Aquablink crack is completely unnecessary now. Thanks!
Comment 5 Tor Lillqvist 2018-02-01 13:55:42 UTC
I am told on IRC that OS X stopped having pulsating default buttons only in 10.10, and as LO supports 10.9 or newer, possibly the code is actually useful still. (But I doubt those who do run LO on 10.9 would be awfully disappointed if the blinking disappeared... Also, we could well bump the minimum OS version to 10.10 by LO 6.1, IMHO.)
Comment 6 Telesto 2018-02-12 19:27:19 UTC
*** Bug 104030 has been marked as a duplicate of this bug. ***
Comment 7 Telesto 2018-11-20 10:40:41 UTC
(In reply to Tor Lillqvist from comment #5)
> I am told on IRC that OS X stopped having pulsating default buttons only in
> 10.10, and as LO supports 10.9 or newer, possibly the code is actually
> useful still. (But I doubt those who do run LO on 10.9 would be awfully
> disappointed if the blinking disappeared... Also, we could well bump the
> minimum OS version to 10.10 by LO 6.1, IMHO.)

Any change to get a bump the minimum OS version to 10.10 or even 10.11 for LO6.3?
Mavericks getting old by now.. Not that I'm in a hurry..

And maybe a bump in compiler baseline too.. even if this isn't needed compiler wise (http://document-foundation-mail-archive.969070.n3.nabble.com/Compiler-baselines-was-Libreoffice-qa-minutes-of-ESC-call-td4244437.html#a4248710)

The last XCode bump did break some stuff on MacOS side (if I remember correctly; page orientation while printing).
Comment 8 Tor Lillqvist 2018-11-20 10:45:06 UTC
Bumping the minimum required version to 10.10 or 10.11 would be fine with me, but I don't decide these things.
Comment 9 Commit Notification 2018-11-27 08:36:22 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/c76160c6e23f8a4e8d60068b7054591f238cbc9f%5E%21

tdf#114839: Drop ancient AquaBlinker crack

It will be available in 6.3.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 10 Xisco Faulí 2018-11-27 09:26:15 UTC
@Tor Lillqvist, thanks for fixing this issue. Should it be cherry-picked to 6.2 branch ?
Comment 11 Tor Lillqvist 2018-11-27 10:59:59 UTC
Well, I guess at least the bump of minimum macOS to 10.9 can not be done for 6.2, as it would come as a SURPRISE to both the LibreOffice users who still run 10.9, and we can't have that, can we? So in theory, assuming that the "pulsating" thing actually works on 10.9 thanks to this code that was removed, removing it in 6.2 would be a REGRESSION!!!