Review Request: Make the rename dialog modeless

Daniele Elmo Domenichelli daniele.domenichelli at gmail.com
Mon Dec 19 11:46:58 UTC 2011


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103453/#review9074
-----------------------------------------------------------


This will conflict with my cumulative patch https://git.reviewboard.kde.org/r/103466 :P
Anyway, let's get this in first, I will rebase mine later...

A few comments:


src/handle-incoming-file-transfer-channel-job.h
<http://git.reviewboard.kde.org/r/103453/#comment7504>

    This method does not need to be started on a different thread (using QTimer) so it does not need to be a slot (in my patch I'm removing some more of those)



src/handle-incoming-file-transfer-channel-job.cpp
<http://git.reviewboard.kde.org/r/103453/#comment7508>

    Please rename it __k__onRenameDialogFinished(), since I'm adding a __k__onResumeDialogFinished() in my patch



src/handle-incoming-file-transfer-channel-job.cpp
<http://git.reviewboard.kde.org/r/103453/#comment7505>

    rename it setUri()



src/handle-incoming-file-transfer-channel-job.cpp
<http://git.reviewboard.kde.org/r/103453/#comment7503>

    modal is false by default if you start it with show(), so this line is useless



src/handle-incoming-file-transfer-channel-job.cpp
<http://git.reviewboard.kde.org/r/103453/#comment7506>

    just call setUri() here



src/handle-incoming-file-transfer-channel-job.cpp
<http://git.reviewboard.kde.org/r/103453/#comment7507>

    same here


Bug 288559 is not fixed by this patch since the idea was to close the rename dialog when the ft is cancelled (even though at the moment I'm not sure if this is a very good idea).
Anyway, don't close that bug with the commit.

Please fix these as soon as possible because I want to get my patch integrated before the feature freeze :D



- Daniele Elmo Domenichelli


On Dec. 18, 2011, 5:25 p.m., Dominik Cermak wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103453/
> -----------------------------------------------------------
> 
> (Updated Dec. 18, 2011, 5:25 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Make the rename dialog modeless so that filetransfer handler doesn't crash when canceling the transfer through the notification while the dialog is open.
> 
> 
> This addresses bugs 283003 and 288559.
>     http://bugs.kde.org/show_bug.cgi?id=283003
>     http://bugs.kde.org/show_bug.cgi?id=288559
> 
> 
> Diffs
> -----
> 
>   src/handle-incoming-file-transfer-channel-job.h 9cc7ad9023e11f002cdd4ed373059e38c754ae3b 
>   src/handle-incoming-file-transfer-channel-job.cpp 0821081d1dbf8948a070fba157c35c5ca26a4fe9 
> 
> Diff: http://git.reviewboard.kde.org/r/103453/diff/diff
> 
> 
> Testing
> -------
> 
> Canceling the filetransfer through the notification while the rename dialog is open doesn't crash filetransfer handler.
> 
> 
> Thanks,
> 
> Dominik Cermak
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20111219/92d103a1/attachment-0001.html>


More information about the KDE-Telepathy mailing list