Review Request: Support drag and drop file transfer from text-ui

Dan Vratil dan at progdan.cz
Wed Jul 11 09:25:01 UTC 2012



> On July 10, 2012, 10:44 p.m., David Edmundson wrote:
> > lib/chat-widget.cpp, line 141
> > <http://git.reviewboard.kde.org/r/105511/diff/1/?file=72045#file72045line141>
> >
> >     I'm not 100% sure why you need this.
> >     
> >     Surely the drop will propogate to the parent (this) anyway, and we can catch it in our own event() method which we already have?
> >     
> >     I may be wrong, ignore me if I am.
> >

Both QTextEdit and QWebView have their own drop event implementations, so I would have to reimplement the event handlers in both subclasses, but that would lead to code duplication (or at least complication). EventFilters receive events before the actual target object, so we handle it there and don't let it propagate to the target object.


> On July 10, 2012, 10:44 p.m., David Edmundson wrote:
> > lib/chat-widget.cpp, line 370
> > <http://git.reviewboard.kde.org/r/105511/diff/1/?file=72045#file72045line370>
> >
> >     filename isn't unique enough, use the URL as you have it.
> >     
> >     
> >     OR
> >     
> >     simply QList<Tp::ChannelPtr> ?
> >

It's easier to retrieve the pointer from QMap using the file as index. Since only temporary files are stored in the map, the path will always be /tmp/kde-$USER/ so the filename is actually unique enough in this case. The variable should be named tempFilesTransfers though.


> On July 10, 2012, 10:44 p.m., David Edmundson wrote:
> > lib/chat-widget.cpp, line 431
> > <http://git.reviewboard.kde.org/r/105511/diff/1/?file=72045#file72045line431>
> >
> >     ...is that true? What if it's a mimetype we can't decode?

You can either drop text/plain or text/html, which we display directly. You can drop raw image data, which we write to tmp file and send or you can drop _any_ file and we send it. 

In general, it's impossible (and extremely unlikely anyone would do it anyway) to drop raw binary data (it would most probably be sent as text/plain anyway).

...but I can be wrong of course.


> On July 10, 2012, 10:44 p.m., David Edmundson wrote:
> > lib/chat-widget.cpp, line 397
> > <http://git.reviewboard.kde.org/r/105511/diff/1/?file=72045#file72045line397>
> >
> >     I think (though could be wrong) this won't work very well.
> >     
> >     A good web browser will fill in both the text and HTML part of the mimedata and thus never trigger this.
> >     
> >     If it did.. the message you send will be full of HTML code, which would look pretty bad to the user the other side.
> >     
> >     (it wouldn't render like HTML they'd see all the "<h1>" type tags.)
> >     
> >     *shrug* after merging we can experiment like crazy and find out what doesn't work.

QTextEdit::insertHtml() \o/


- Dan


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


On July 10, 2012, 9:32 p.m., Dan Vratil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105511/
> -----------------------------------------------------------
> 
> (Updated July 10, 2012, 9:32 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> - dropping a local URL starts a file transfer
> - dropping a remote URL appends it to the SendMessageBox
> - dropping text or HTML data appends it to the SendMessageBox
> - dropping image data creates a temporary file (PNG), starts a file transfer and when it's finished the file is removed again
> 
> 
> This addresses bug 288561.
>     http://bugs.kde.org/show_bug.cgi?id=288561
> 
> 
> Diffs
> -----
> 
>   app/main.cpp b8b2523 
>   lib/chat-text-edit.h 3e2dec0 
>   lib/chat-text-edit.cpp 2985aee 
>   lib/chat-widget.h 686b632 
>   lib/chat-widget.cpp e3439d8 
> 
> Diff: http://git.reviewboard.kde.org/r/105511/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Vratil
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120711/4f75de17/attachment.html>


More information about the KDE-Telepathy mailing list