DevHeads.net

Review Request: KWallet Password Prompt Dialog In Your Face

Review request for KDE Runtime, David Faure and Fredrik Höglund.

Description
This is an attempt to make the KWallet password prompt much harder to ignore or miss.

Now the prompt should always be in front of the parent window. and it should unminimize if needed, and demand attention.

Diffs
kwalletd/kwalletd.cpp 309c45f

Diff: <a href="http://git.reviewboard.kde.org/r/105628/diff/" title="http://git.reviewboard.kde.org/r/105628/diff/">http://git.reviewboard.kde.org/r/105628/diff/</a>

Testing
Just using it in various scenarios.
For example, if the akonadi maildispatcher needs to open kwallet now the password prompt is always in front of kmail

Thanks,

Allen Winter

Comments

Re: Review Request: KWallet Password Prompt Dialog In Your Face

By Allen Winter at 08/04/2012 - 08:35

(Updated Aug. 4, 2012, 12:35 p.m.)

Review request for KDE Runtime, David Faure and Fredrik Höglund.

Description (updated)
This is an attempt to make the KWallet password prompt much harder to ignore or miss.

Now the prompt should always be in front of the parent window. and it should unminimize if needed, and demand attention.

[UPDATE] I'm reopening as I still think this is needed, even in 4.9.
I changed the things that gave people heartburn in my first attempt, notably removing the usertime=0

Diffs (updated)
kwalletd/kwalletd.cpp 309c45f

Diff: <a href="http://git.reviewboard.kde.org/r/105628/diff/" title="http://git.reviewboard.kde.org/r/105628/diff/">http://git.reviewboard.kde.org/r/105628/diff/</a>

Testing (updated)
Just using it in various scenarios.
For example, if the akonadi maildispatcher needs to open kwallet now the password prompt is always in front of kmail

[UPDATE] Without the usertime=0, you can still move kmail in front of the password prompt dialog. but at least you see the prompt first.

Thanks,

Allen Winter

Re: Review Request: KWallet Password Prompt Dialog In Your Face

By Commit Hook at 08/18/2012 - 15:08

This review has been submitted with commit 3cee7d5d8f185a7b11d64ba3b2094db78e3d855a by Allen Winter to branch KDE/4.9.

- Commit Hook

On Aug. 4, 2012, 12:35 p.m., Allen Winter wrote:

Re: Review Request: KWallet Password Prompt Dialog In Your Face

By Commit Hook at 08/18/2012 - 15:07

This review has been submitted with commit 9c38b48fb37747de3708a75d73c1f428ece72100 by Allen Winter to branch master.

- Commit Hook

On Aug. 4, 2012, 12:35 p.m., Allen Winter wrote:

Re: Review Request: KWallet Password Prompt Dialog In Your Face

By Martin =?iso-88... at 08/15/2012 - 15:15

this approach seems fine to me.

- Martin Gräßlin

On Aug. 4, 2012, 12:35 p.m., Allen Winter wrote:

Re: Review Request: KWallet Password Prompt Dialog In Your Face

By Allen Winter at 08/16/2012 - 19:12

Thanks for the feedback Martin.
I will commit within a couple days unless someone screams.

- Allen

On Aug. 4, 2012, 12:35 p.m., Allen Winter wrote:

Re: Review Request: KWallet Password Prompt Dialog In Your Face

By =?utf-8?q?Thoma... at 07/20/2012 - 10:17

The point is that one process (akonadi) shall be able to get the dialog on top of a random different (probably fullscreen?) window, yesno?

KWindowSystem::forceActiveWindow(wId);
KWindowSystem::raiseWindow(wId);

alone will do that - but as Christoph and Martin pointed out: it's an incredibly nasty way - you're abusing tools meant for pagers and taskbars here.

kwalletd/kwalletd.cpp
<http://git.reviewboard.kde.org/r/105628/#comment12741>

"activeDialog()->show()" and that's (unless it's reparented) the same as ...

kwalletd/kwalletd.cpp
<http://git.reviewboard.kde.org/r/105628/#comment12743>

What's the point of demanding attention when the very next step is to force the window to be active what will clear that state - confuse libtaskbar?

kwalletd/kwalletd.cpp
<http://git.reviewboard.kde.org/r/105628/#comment12744>

Faking the usertime stamp is only good to cheat the focus stealing prevention, but you're invoking the tool flag ("forceActive") what completely bypasses the FSP anyway

kwalletd/kwalletd.cpp
<http://git.reviewboard.kde.org/r/105628/#comment12742>

... unminimizeWindow what's the same as activeDialog->show()

- Thomas Lübking

On July 20, 2012, 1:41 p.m., Allen Winter wrote:

Re: Review Request: KWallet Password Prompt Dialog In Your Face

By =?utf-8?q?Thoma... at 07/20/2012 - 11:13

The call is pointless - as soon as the window gets focus urgency *should* be withdrawn.
If your taskbar still flashes, that's a bug in the taskbar.

KWindowSystem::forceActiveWindow(wId);
KWindowSystem::raiseWindow(wId);

that order (first breaks

It's still wrong (but less wonky than playing on the user time to fake interaction)

If we need an immediate hack the better way would be to autoship a window rule, but that only holds on kwin and is still a hackish solution to avoid facing a conceptual defect.

- Thomas

On July 20, 2012, 1:41 p.m., Allen Winter wrote:

Re: Review Request: KWallet Password Prompt Dialog In Your Face

By Allen Winter at 07/20/2012 - 11:05

probably don't need the demand attention. but i don't think it hurts

usertime is the only way I could find so that the prompt dialog couldn't be hidden by the main kmail window.

- Allen

On July 20, 2012, 1:41 p.m., Allen Winter wrote:

Re: Review Request: KWallet Password Prompt Dialog In Your Face

By Martin =?iso-88... at 07/20/2012 - 10:14

kwalletd/kwalletd.cpp
<http://git.reviewboard.kde.org/r/105628/#comment12745>

from API docs: "The usage of forceActiveWindow() is meant only for pagers and similar tools, which represent direct user actions related to window manipulation. Except for rare cases, this request will be always honored, and normal applications are forbidden to use it."

I do not see that this is a pager or a taskbar. This means forceActiveWindow is the wrong way to do it.

The way it is done here is an example of how to do a nag window. Please make it just a transient to the window it belongs to. KWin should take care of presenting it correctly. It is nothing which has to be above everything else. If we want that we should do it correctly, that is really block till the user entered the password. But in general I'm against windows behaving like that. It would make it very difficult for users to figure out what's going on and I think it's a very bad idea to ask for passwords in a nag window way as long as we have "kded asks for password". Yeah right, what's kded again?

- Martin Gräßlin

On July 20, 2012, 1:41 p.m., Allen Winter wrote:

Re: Review Request: KWallet Password Prompt Dialog In Your Face

By Martin =?iso-88... at 07/20/2012 - 11:48

It's not possible to force one window to have focus till the user dealt with it and I hope we all see that we don't want to implement support for that.

But I think we are trying here to add further glue to something which is completely broken from the ground up. I'll send a mail to kcd and plasma suggesting a completely different approach.

- Martin

On July 20, 2012, 1:41 p.m., Allen Winter wrote:

Re: Review Request: KWallet Password Prompt Dialog In Your Face

By Allen Winter at 07/20/2012 - 11:01

So I don't know much about window managers and stuff.. what I want is to make sure that no matter what the prompt dialog is in the user's face and can't be dismissed.

The worst case I've found is when the akonadi mail dispatcher needs to open the wallet. the prompt dialog for that has been hidden in some cases.. and can also be stacked below kmail or minimized. when that happens the user sees their message sitting in their outbox forever (since the mail dispatcher is waiting for kwallet to return)

forceActiveWindow() has always been there. my patch did not change that.
I added unminimize and setUserTime (and demandsattention)

- Allen

On July 20, 2012, 1:41 p.m., Allen Winter wrote:

Re: Review Request: KWallet Password Prompt Dialog In Your Face

By =?utf-8?q?Thoma... at 07/20/2012 - 10:24

There're two issues here
a) they don't know the main window
b) it can happen that the wallet password dialog is invoked by two applications at the same time and you can't have one dialog transient for two main windows (for good reason, btw - that's never gonna be "fixed")

So tasks are:
a) figure the mainwindow for daemons (hard enough - what if some plasmoid & kmail access akonadi?)
b) figure to which one the transient should be applied if there's multiple kwalled access

Optionally: question whether the triggered invocation is an ideal approach at all (-> single login, seems heftily demanded)

- Thomas

On July 20, 2012, 1:41 p.m., Allen Winter wrote:

Re: Review Request: KWallet Password Prompt Dialog In Your Face

By Christoph Feck at 07/20/2012 - 10:04

Well, I already mentioned it on IRC, but what you do is not "keep KWallet above KMail", but "keep KWallet above all windows".

Let's say, I start KMail. Since it needs a lot of time to start, I already switch to another window to work somewhere else. When KMail later asks for KWallet, it will interrupt me there.

I understand that it is much harder to fix it correctly (basically KMail would have to register itself as the current "Akonadi UI", so that all agents know where to put the window). I fear that once this patch is in, no further attempt is done. While it's nice that we separate components in KDE, we should make sure they still act integrated.

- Christoph Feck

On July 20, 2012, 1:41 p.m., Allen Winter wrote: