Bug 265738

Summary:	Multiple issues with Applet::showMessage()
Product:	[Unmaintained] plasma4	Reporter:	Martin Blumenstingl <martin.blumenstingl>
Component:	general	Assignee:	Plasma Bugs List <plasma-bugs>
Status:	RESOLVED FIXED
Severity:	normal
Priority:	NOR
Version:	unspecified
Target Milestone:	---
Platform:	Arch Linux
OS:	Linux
Latest Commit:		Version Fixed In:	4.6.1
Sentry Crash Report:

Description Martin Blumenstingl 2011-02-07 23:24:02 UTC

Version: unspecified (using KDE 4.6.0)
OS: Linux

There are multiple issues with Applet::showMessage().

1) This change broke API compatibility: https://projects.kde.org/projects/kde/kdelibs/repository/revisions/4eedd723a8f8bcb250b65d8f0a4aa3bd58f4ce20
Consumers of Plasma::Applet connected to messageButtonPressed() with "MessageButton" as signature -> now one has to prefix that with the namespace.

2) Pressing the escape button when a message is shown does not emit "messageButtonPressed". API consumers should somehow be able to detect that the message is not shown anymore.

3) I have a message with two buttons: "Plasma::ButtonYes | Plasma::ButtonNo". No matter which button I click, plasma always emits messageButtonPressed( Plasma::ButtonNo ).

4) Another minor issue is that the "Cancel" button does not work correctly.
The reason is quite obvious if you look at the source code: https://projects.kde.org/projects/kde/kdelibs/repository/revisions/master/entry/plasma/applet.cpp#L657

Issues 1-3 seem to be regressions in KDE 4.6.

Reproducible: Always

Steps to Reproduce:
One way to reproduce issues 1-3 is to use amarok's lyrics applet.

Expected Results:
1) Signatures should not change. An easy fix for this may be introducing a "backwards compatibility signal".

2) AppletPrivate::destroyMessageOverlay() should check if the sender was the "Escape key" action and emit Plasma::ButtonCancel.

3) When clicking "Yes" plasma should emit Plasma::ButtonYes ;)

4) The fix for this is probably a nobrainer :)

Comment 1 Marco Martin 2011-02-09 00:13:06 UTC

Git commit c9256561d74c9e890c061f4c289a4437a125953d by Marco Martin.
Committed on 09/02/11 at 00:02.
Pushed by mart into branch 'KDE/4.6'.

emit cancel signal

both when clicking cancel and pressing esc
CCBUG:265738

M  +7    -4    plasma/applet.cpp     
M  +1    -0    plasma/private/applet_p.h     

http://commits.kde.org/kdelibs/c9256561d74c9e890c061f4c289a4437a125953d

Comment 2 Marco Martin 2011-02-09 00:13:07 UTC

Git commit e9332d4825ca9ee65b2d03c5a4a8ef761fb9c301 by Marco Martin.
Committed on 09/02/11 at 00:02.
Pushed by mart into branch 'master'.

emit cancel signal

both when clicking cancel and pressing esc
CCBUG:265738

M  +7    -4    plasma/applet.cpp     
M  +1    -0    plasma/private/applet_p.h     

http://commits.kde.org/kdelibs/e9332d4825ca9ee65b2d03c5a4a8ef761fb9c301

Comment 3 Marco Martin 2011-02-09 19:21:52 UTC

Git commit 6a4bf6e507e5a084ab52a5a7ced1a8cbb70187a8 by Marco Martin.
Committed on 09/02/11 at 19:17.
Pushed by mart into branch 'KDE/4.6'.

correct the text used to fugure out the button

button have text like &Ok &Yes
This way to detect what button was pressed of course is horrible and will have to be changed
CCBUG:265738

M  +4    -4    plasma/applet.cpp     

http://commits.kde.org/kdelibs/6a4bf6e507e5a084ab52a5a7ced1a8cbb70187a8

Comment 4 Marco Martin 2011-02-09 19:21:52 UTC

Git commit 356560329763664fbbdd97b17401a23fca5f9da8 by Marco Martin.
Committed on 09/02/11 at 19:17.
Pushed by mart into branch 'master'.

correct the text used to fugure out the button

button have text like &Ok &Yes
This way to detect what button was pressed of course is horrible and will have to be changed
CCBUG:265738

M  +4    -4    plasma/applet.cpp     

http://commits.kde.org/kdelibs/356560329763664fbbdd97b17401a23fca5f9da8

Comment 5 Martin Blumenstingl 2011-02-09 21:16:09 UTC

Issues 2-4 are fixed (thanks notmart!)

The solution for the first issue still has to be decided.
Discussion can be followed here: http://mail.kde.org/pipermail/plasma-devel/2011-February/014997.html

Comment 6 Martin Blumenstingl 2011-02-19 23:14:01 UTC

Fixed.

It was decided that the Plasma namespace prefix will stay.
I already added a fix for issue #1 (the signature issue) in amarok - so I'm fine with that decision.