> On June 25, 2013, 6:46 a.m., Kevin Ottens wrote: > > kio/kfile/kurlrequester.cpp, line 483 > > <http://git.reviewboard.kde.org/r/111184/diff/2/?file=165624#file165624line483> > > > > Why don't you use KUrlMimeData like the original code in KLineEdit? > > It'll be more robust AFAICT. > > Albert Vaca Cintora wrote: > I'm using hasUrls() instead because KUrlMimeData returns a list of urls, > which is more expensive than what Qt does (a string comparison). > > Also, if our way of checking URLs is better than the way Qt uses, we > should contribute it to Qt instead of avoid using their methods (we are going > to rely a lot more on Qt since KF5, so we have to trust what Qt does). > > Kevin Ottens wrote: > Well, KUrlMimeData has some specific handling for KIO, which is a big > no-no for Qt. That's more what I had in mind... we don't need the metadata > part in that context though, so it's likely a moot point. > > That said we look for the application/x-kde4-urilist mimetype too (again > something we can't really push in Qt). David, any reason why we have this > extra mimetype? Doesn't really makes sense to me.
KUrlMimeData has some specific handling for KIO, and not just metadata, but also shipping two lists of URLs: KIO urls and most-local urls. For instance desktop:/foo and file://home/dfaure/Desktop/foo, the first one being aimed at KIO-enabled apps and the second one at non-KIO-enabled apps. The code here simply checks for "there are urls", and lets QLineEdit do the extraction. This means that the most-local urls are going to be pasted, not the KIO ones. But that's not really specific to kurlrequester nor the clear-before-insert feature... (e.g. konqueror's location bar would have the same issue) WAIT ..... who determined that only KUrlRequester used the "url drop" feature of KLineEdit? That feature is ON by default, so looking for calls to setUrlDropsEnabled(true) is definitely wrong. Konqueror's location bar definitely uses that feature too. I suspect the location bar in dolphin and the one in kfiledialog do as well, at least. It seems to me that putting the code into KUrlRequester is just wrong. It should be an event filter that can be plugged onto any QLineEdit. And then it can take care of both: clear before insert, and using KUrlMimeData to do the URL extraction. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111184/#review35019 ----------------------------------------------------------- On June 25, 2013, 10:46 a.m., Albert Vaca Cintora wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111184/ > ----------------------------------------------------------- > > (Updated June 25, 2013, 10:46 a.m.) > > > Review request for KDE Frameworks. > > > Description > ------- > > KLineEdit and KComboBox had the property enableUrlDrops that was added to use > it in KUrlRequester. Since KUrlRequester is part of KIO, it can not use > Kde4Support and should be ported to use QLineEdit and QComboBox. As I said on > my email to the frameworks list, it was easier to implement this directly in > KUrlRequester than patching Qt, even though it was originally planned as a Qt > task. It was my intention to also remove that property from KLineEdit and > KComboBox, but finally I didn't include it in the patch (see Ervin's comment > in the mailing list). > > In the patch I have also added a TODO because I think it would be good to > detect any url without needing the mimetype to be set, but I have not done it > yet because it should be discussed first. > > (Reviewed by aacid and apol) > > > Diffs > ----- > > KDE5PORTING.html ba67bdc > kdeui/widgets/kcombobox.h ccb019d > kdeui/widgets/klineedit.h 7ac22f6 > kio/kfile/kurlrequester.cpp fa6b234 > > Diff: http://git.reviewboard.kde.org/r/111184/diff/ > > > Testing > ------- > > > Thanks, > > Albert Vaca Cintora > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
