> On Jan. 13, 2015, 12:41 nachm., Christoph Feck wrote: > > It looks like Daniel cannot provide feedback right now. > > > > Wolfgang, does changing the modality of the configure dialog mean it can > > now be openend twice? > > Wolfgang Bauer wrote: > As far as I can tell, no. > If I click on "Configure" for the same printer, the already opened dialog > is given focus, no new dialog is opened. > > It is possible to open a new configure dialog for a different printer now > though when one is open already, which even makes sense somehow IMHO. > > Thomas Lübking wrote: > A modal window is ALWAYS modal for exactly one other window. > The assumption > "Launching a modal dialog (for example, as password dialog) from a modal > dialog won't give the focus to the second dialog." in bug #314633 is > -generally- wrong for sure. > > Beyond this, Qt supports client wide modality, but that's about event > processing. > > You can set QWidget::setWindowModality(Qt::WindowModal) rather than > QWidget::setModal(true) (the latter implies > QWidget::setWindowModality(Qt::ApplicationModal) to impact this. > > Notice that Qt will set the main window to the parent window - you must > use KWindowSystem::setMainWindow(QWidget*, WId) if you need anything special. > > KWin (unlike mutter) does not support some sort of system wide modality > at all. I cannot say what exactly causes the behavior of bug #314633 but a > modal dialog for MainWindow A has no impact on windows in clients B unless A > and B have a common MainWindow C which is NOT the root window. > > > ==> The bug is here: > > https://projects.kde.org/projects/kde/kdeutils/print-manager/repository/revisions/1595ef0614f824a6a871d51327eff3a108e6e251 > > Why would the password dialog block plasma? Because it's ApplicationModal > or because it was set modal for some common plasma dialog dummy window? > If the dialog has an actual (visible, real, probably not the desktop or a > panel) parent window, it should be WindowModal for that one window only. (I > strongly assume the the password dialog better should be modal since its > input is probably expected by some other window?!) If not (ie. the dialog > spawns out of the void), then of course it should be not modal at all. > > Wolfgang Bauer wrote: > I fully agree that the password dialog should better be modal. > But as I understand bug #314633, the password dialog can also be shown > directly by the plasmoid, i.e. it _can_ spawn out of the void apparently. > I never saw that problem though (I don't use remote printers), and AFAICS > the real cause was never identified sufficiently. > > OTOH, I don't really see a point for making the "Configure Printer" > dialog modal. > Why should it be prevented to do some other stuff in the KCM while that > dialog is open, like cleaning the heads or printing a test page. > > Thomas Lübking wrote: > Modality should be client controlled then (ie. the printer dialog would > set it QWidget::setWindowModality(Qt::WindowModal) - it MUST be a child of > the printer dialog, resp. through KWindowSystem::setMainWindow(passwordDlg, > printerDlg->winId()) ) > > I would instinctively agree on the printer dialog modality, but can't say > - I installed printer stuff only back then to check the modality > constallation but am not in tree killing business otherwise ;-) > > Wolfgang Bauer wrote: > So if I understand you correctly, > dialog->setWindowModality(Qt::WindowModal) should be called instead of > dialog->setModal(), and then KWindowSystem::setMainWindow() to specify the > window that should be blocked (i.e. the "Configure Printer" dialog in this > case). > > AFAICS KWindowSystem::setMainWindow() is actually called already (added > by the mentioned commit), so all that would be left is change > dialog->setModal() to dialog->setWindowModality(Qt::WindowModal) in > KCupsPasswordDialog.cpp? > > But KWindowSystem::setMainWindow() should be called *before* > dialog->show() (not afterwards as it is now) if I correctly understand the > documentation, right? > > And I think the KWindowSystem::forceActiveWindow(dialog->winId()) should > be removed as well, or not? > > --- > > Regarding *this* review request: configure-printer (i.e. the printer > configuration dialog) is a separate application anyway > (/usr/lib64/kde4/libexec/configure-printer on my system, it is started via > DBUS). So the setModal(true) here is totally useless I'd say. > It has absolutely no effect on the KCM window at all, that one is not > blocked even without this patch. I noticed this already when I came up with > this patch, but somehow forgot to mention that here. > > So I think there's no need to discuss whether the "configure printer" > dialog should be modal or not.
PS: I did notice a change now: When setModal(true) is called, all other open "Configure Printer" dialog windows are blocked when you open a new one. I'm not sure whether this is desirable or not though. But I'd rather say no. It should not matter in which order you press OK/Cancel in the open windows I think... - Wolfgang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121351/#review73924 ----------------------------------------------------------- On Jan. 13, 2015, 12:40 nachm., Wolfgang Bauer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121351/ > ----------------------------------------------------------- > > (Updated Jan. 13, 2015, 12:40 nachm.) > > > Review request for Okular, Print Manager, Daniel Nicoletti, and Thomas > Lübking. > > > Bugs: 328014 > http://bugs.kde.org/show_bug.cgi?id=328014 > > > Repository: print-manager > > > Description > ------- > > [Commit > 1595ef0614f824a6a871d51327eff3a108e6e251](https://projects.kde.org/projects/kde/kdeutils/print-manager/repository/revisions/1595ef0614f824a6a871d51327eff3a108e6e251) > (a partial fix for bug 314633) changed the password dialog to be non-modal. > But the printer configuration dialog is still modal, so if a password is > needed to apply/save the configuration it cannot be entered because the > password dialog cannot get focus. > > This patch sets the configuration dialog to be non-modal as well to prevent > this problem. > > > Diffs > ----- > > configure-printer/ConfigureDialog.cpp ace91a2 > > Diff: https://git.reviewboard.kde.org/r/121351/diff/ > > > Testing > ------- > > Open the "Printer" KCM in systemsettings, select a printer, click on > "Configure", change some setting and click "Apply" or "OK". > A password dialog appears (unless you have the necessary privileges to change > the CUPS settings of course), this has focus and you can actually enter the > username/password now whereas you could not without this patch. > > Also tested by other users and included in openSUSE's official packages, see > https://bugzilla.opensuse.org/show_bug.cgi?id=889187#c7. > > > Thanks, > > Wolfgang Bauer > >
_______________________________________________ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
