Bug 117280 - Duplicate Macro execution when triggered by Document print event.
Summary: Duplicate Macro execution when triggered by Document print event.
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
4.2 all versions
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.4.0 target:7.3.0.0.beta2
Keywords: bibisected, bisected, regression
Depends on:
Blocks: Macro
  Show dependency treegraph
 
Reported: 2018-04-27 09:24 UTC by Thierry Jeanneret
Modified: 2022-03-22 08:27 UTC (History)
4 users (show)

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


Attachments
Document containing a demo macro hooked to the print event (10.93 KB, application/vnd.oasis.opendocument.text)
2018-04-27 09:43 UTC, Thierry Jeanneret
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thierry Jeanneret 2018-04-27 09:24:49 UTC
Hello,

When I hook a BASIC macro to the "Print the Document" event the macro is executed twice.

I tried to simplify the macro to its minimum, by just launching a msgBox Saying "Macro", the behaviour is the same.

Thierry
Comment 1 Xisco Faulí 2018-04-27 09:26:02 UTC
Thank you for reporting the bug. Please attach a sample document, as this makes it easier for us to verify the bug. 
(Please note that the attachment will be public, remove any sensitive information before attaching it. 
See https://wiki.documentfoundation.org/QA/FAQ#How_can_I_eliminate_confidential_data_from_a_sample_document.3F for help on how to do so.)

I have set the bug's status to 'NEEDINFO'. Please change it back to 'UNCONFIRMED' once the requested document is provided.
Comment 2 Thierry Jeanneret 2018-04-27 09:43:12 UTC
Created attachment 141697 [details]
Document containing a demo macro hooked to the print event
Comment 3 Xisco Faulí 2018-04-27 10:05:10 UTC
Actually, the dialog is displayed 3 times in

Version: 6.1.0.0.alpha1+
Build ID: 653e58f9eb3d4ee61d8103993cdff2660c9127a5
CPU threads: 4; OS: Linux 4.13; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group

2 times in

Version: 6.0.3.2
Build ID: 1:6.0.3~rc2-0ubuntu0.16.04.1~lo2
CPU threads: 4; OS: Linux 4.13; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group
Comment 4 Xisco Faulí 2018-04-27 10:10:10 UTC
Reproduced in

Version: 5.2.0.0.alpha0+
Build ID: 3ca42d8d51174010d5e8a32b96e9b4c0b3730a53
Threads 4; Ver: 4.13; Render: default;

and

Version: 4.3.0.0.alpha1+
Build ID: c15927f20d4727c3b8de68497b6949e72f9e6e9e
Comment 5 Xisco Faulí 2018-04-27 10:22:35 UTC
The dialog being displayed 2 times started after

author	Ariel Constenla-Haile <arielch@apache.org>	2013-02-24 18:23:21 +0000
committer	Caolán McNamara <caolanm@redhat.com>	2013-06-19 15:46:54 +0100
commit 87dfa6dd336d596112c0beb6b42f082178461678 (patch)
tree b3bde0c7c200ac231a522e496213ad38e65337e5
parent bf4ecd6138f07ca6207eeec306517eff4aff220e (diff)
Resolves: #i121810# Adapt SfxPrintingHint to work with...
the "new" XDocumentEventBroadcaster

(cherry picked from commit 1bfae56dd9d633a80924bfeefc03368100d75a8f)

Conflicts:
	sfx2/inc/sfx2/event.hxx
	sfx2/source/view/viewprn.cxx

Bisected with bibisect-42max

and the problem with the dialog being displayed 3 times started after

author	Julien Nabet <serval2412@yahoo.fr>	2018-01-30 21:34:52 +0100
committer	Noel Grandin <noel.grandin@collabora.co.uk>	2018-02-01 15:47:22 +0100
commit 4b2476b5ac464d876cb1be4d3e02e30c329607cc (patch)
tree 157ed4bb5beed33129560a5e5b4d11194bb6ac36
parent ae8b65eea784e4f718e3170577b670fe1e6a3700 (diff)
Avoid empty avoid name for SfxPrintingHint
Create a brand new doc on Writer, click Print then Cancel button
=>
warn:legacy.tools:2382:2382:sfx2/source/doc/sfxbasemodel.cxx:3071: Empty event name!

Bisected with: bibisect-linux64-6.1

Adding Cc: to Julien Nabet
Comment 6 Julien Nabet 2018-04-28 17:13:39 UTC Comment hidden (obsolete)
Comment 7 Julien Nabet 2018-05-04 21:36:52 UTC
On pc Debian x86-64 with master sources updated today, I confirm I got 3 times the message.
I didn't try with reverting locally my commit because since the file changed is in "include/sfx2", it takes some time to just build.
Again, feel free to revert the patch if you want.
Comment 8 QA Administrators 2021-08-13 03:54:50 UTC Comment hidden (obsolete, spam)
Comment 9 Justin L 2021-11-24 10:47:16 UTC
repro 7.3+ with three message boxes.
I expect that the macro hook should be watching a different event, but I know nothing at all about that code.
Comment 10 Julien Nabet 2021-11-28 13:55:54 UTC
I retrieved 3 bts (1 per message displayed).
All of them come from sfx2/source/view/viewprn.cxx

The first 2 ones are in SfxPrinterController::jobStarted()
1)
316     SfxGetpApp()->NotifyEvent( SfxEventHint(SfxEventHintId::PrintDoc, GlobalEventConfig::GetEventName( GlobalEventId::PRINTDOC ), mpObjectShell ) );

2)
324     mpObjectShell->Broadcast( SfxPrintingHint(
325         view::PrintableState_JOB_STARTED, aOpts, mpObjectShell, xController ) );

the last one is in SfxPrinterController::jobFinished
334     mpObjectShell->Broadcast( SfxPrintingHint( nState ) );
Comment 11 Julien Nabet 2021-11-28 14:38:47 UTC
(In reply to Julien Nabet from comment #10)
> I retrieved 3 bts (1 per message displayed).
> ...

The 2 last ones are due to the already quoted commits.
In include/sfx2/event.hxx

The 2 ctrs of SfxPrintingHint call SfxViewEventHint with second arg
"GlobalEventConfig::GetEventName( GlobalEventId::PRINTDOC )"
See https://opengrok.libreoffice.org/xref/core/include/sfx2/event.hxx?r=503e0173#239.

The pb is we got several printable states:
- JOB_STARTED
- JOB_COMPLETED
etc.
and only 1 event onPrint
see https://opengrok.libreoffice.org/xref/core/offapi/com/sun/star/view/PrintableState.idl?r=2b383d19#31

At which moment should we trigger the event when job began, when it finished?

Also, I don't know event mechanism to distinguish what SfxGetpApp()->NotifyEvent compared to mpObjectShell->Broadcast
Perhaps the initial call "SfxGetpApp()->NotifyEvent..." could be removed?

Mike: any opinion here or any idea whom may help here?
Comment 12 Mike Kaganski 2021-11-29 06:12:17 UTC
I suppose that we just need to supply a proper data to the Supplement member of com.sun.star.document.DocumentEvent struct getting passed as an argument to the event handler. It could be a string, or an integer constant, so that the handler could differentiate the events.

(In reply to Julien Nabet from comment #11)
> The pb is we got several printable states:
> - JOB_STARTED
> - JOB_COMPLETED
> etc.
> and only 1 event onPrint

The approach above allows to address that. com.sun.star.view.PrintableState enum looks a natural choice here... unless we want something more powerful.

I assume that we actually want two events (before and after) out of three, and the first one maybe can go. But the two calls in SfxPrinterController::jobStarted appear before and after calling getJobProperties - maybe that has a value, and we only need to clarify what options are available for developer at each call (and then we definitely need something else than com.sun.star.view.PrintableState there, since there's no values to disambiguate the two; and actually, I don't know what user can really do with the getJobProperties, so the above is just a guess). Also the two methods not only call Basic methods, but do other work - I would expect problems simply removing the calls from here - we'd need to provide a way for passing the information "do not call events" there, e.g. setting a SfxApplication flag, or pushing a context (see <comphelper/SetFlagContextHelper.hxx>).
Comment 13 Julien Nabet 2021-11-29 09:20:55 UTC
Thank you Mike for your feedback!
I gave a try with https://gerrit.libreoffice.org/c/core/+/126018 as you may have seen.
Comment 14 Julien Nabet 2021-11-29 12:03:21 UTC
My patch was wrong and I don't understand all the stuff with supplement and context => unassign myself and from this bugtracker.

Sorry for the noise Mike (don't hesitate to uncc of course).
Comment 15 Commit Notification 2021-11-29 16:22:33 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/774c6a6e1603bf3f12f1573b0778e0f0f9783169

tdf#117280: fix multiple Macro execution when triggered by Document print event

It will be available in 7.4.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 16 Commit Notification 2021-11-29 20:37:35 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

https://git.libreoffice.org/core/commit/20f56c937c44a9cbe3c05fa7c510feaf9d2341df

tdf#117280: fix multiple Macro execution when triggered by Document print event

It will be available in 7.3.0.0.beta2.

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 17 Commit Notification 2021-11-30 05:21:50 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/28eef82cb16faef0b8ddc9912560efb779baa9f9

tdf#117280: derive SfxEvents_Impl from css::document::XDocumentEventListener

It will be available in 7.4.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 18 Mike Kaganski 2021-11-30 06:41:23 UTC
FTR: The solution implemented in the abovementioned commits is to pass com.sun.star.view.PrintableState [1] value in the com.sun.star.document.DocumentEvent struct's Supplement, passed as event handler's argument. So the event handler could look like this:


> Sub OnPrintHandler(e)
>   If (e.Supplement = com.sun.star.view.PrintableState.JOB_STARTED) Then
>     MsgBox("JOB_STARTED")
>   ElseIf (e.Supplement = com.sun.star.view.PrintableState.JOB_COMPLETED) Then
>     MsgBox("JOB_COMPLETED")
>   ElseIf (e.Supplement = com.sun.star.view.PrintableState.JOB_SPOOLED) Then
>     MsgBox("JOB_SPOOLED")
>   ElseIf (e.Supplement = com.sun.star.view.PrintableState.JOB_ABORTED) Then
>     MsgBox("JOB_ABORTED")
>   ElseIf (e.Supplement = com.sun.star.view.PrintableState.JOB_FAILED) Then
>     MsgBox("JOB_FAILED")
>   ElseIf (e.Supplement = com.sun.star.view.PrintableState.JOB_SPOOLING_FAILED) Then
>     MsgBox("JOB_SPOOLING_FAILED")
>   Else
>     MsgBox(e.Supplement)
  EndIf
End Sub

Julien has removed one likely unnecessary notification, so the event is generated twice instead of three times, as was noticed in comment 3. The two other invocations are legitimate, and typically reflect JOB_STARTED then JOB_SPOOLED, or JOB_STARTED then JOB_ABORTED. The handler should handle the kind of event that it needs, based on the PrintableState, as shown above.

Interesting, if this is an API change. Also, it likely needs documentation. Also, it might be possible in the future to re-consider what is put into Supplement field, so user code should be prepared that Supplement may be empty (as prior to the change), or numeric (as now), or maybe something else (a struct?).

[1] https://api.libreoffice.org/docs/idl/ref/namespacecom_1_1sun_1_1star_1_1view.html#ad9b0afaffefc166344fd9575516b6626
Comment 19 Commit Notification 2021-12-09 08:51:47 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

https://git.libreoffice.org/core/commit/7f34ac5201ffec31bd336cf9fb1f8d2554bc6f72

tdf#117280: derive SfxEvents_Impl from css::document::XDocumentEventListener

It will be available in 7.3.0.0.beta2.

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 20 Justin L 2022-02-08 13:27:21 UTC
Mike: should this bug be marked as fixed now?
Comment 21 Mike Kaganski 2022-02-08 13:30:13 UTC
I guess so :-)
Comment 22 Timur 2022-03-21 15:12:22 UTC
Xisco, please retest since you tested before.
Comment 23 Timur 2022-03-22 08:27:22 UTC
I see 2 messages, so not clear if resolved since there used to be 1 up to 4.2, and who will comment on comment 18 and API.