Re: Review Request 119869: KIO: Correctly handle .directory files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119869/ --- (Updated Aug. 22, 2014, 3:48 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, kdelibs and Frank Reininghaus. Bugs: 235457 https://bugs.kde.org/show_bug.cgi?id=235457 Repository: kio Description --- My attempt at fixing [bug 235457](https://bugs.kde.org/show_bug.cgi?id=235457) We give .directory the default Type 'Directory' and a way for KDesktopFileActions to handle .directory files. To do that we trick KRun in opening the file as plain text. hasDirectoryType is a new method in KConfig. see: [119868](https://git.reviewboard.kde.org/r/119868/) Anything I should look out for when commiting this to KConfig and KIO? Diffs - src/widgets/kdesktopfileactions.cpp 9486015 Diff: https://git.reviewboard.kde.org/r/119869/diff/ Testing --- Configure a folder with a custom icon in dolphin. Click the created .directory file. Thanks, Maarten De Meyer
Re: Review Request 119869: KIO: Correctly handle .directory files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119869/ --- (Updated Aug. 21, 2014, 1:15 p.m.) Review request for KDE Frameworks, kdelibs and Frank Reininghaus. Bugs: 235457 https://bugs.kde.org/show_bug.cgi?id=235457 Repository: kio Description --- My attempt at fixing [bug 235457](https://bugs.kde.org/show_bug.cgi?id=235457) We give .directory the default Type 'Directory' and a way for KDesktopFileActions to handle .directory files. To do that we trick KRun in opening the file as plain text. hasDirectoryType is a new method in KConfig. see: [119868](https://git.reviewboard.kde.org/r/119868/) Anything I should look out for when commiting this to KConfig and KIO? Diffs (updated) - src/widgets/kdesktopfileactions.cpp 9486015 Diff: https://git.reviewboard.kde.org/r/119869/diff/ Testing --- Configure a folder with a custom icon in dolphin. Click the created .directory file. Thanks, Maarten De Meyer
Re: Review Request 119868: KConfig add DirectoryType support
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119868/ --- (Updated Aug. 21, 2014, 1:15 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks, kdelibs and Frank Reininghaus. Repository: kconfig Description --- Add a hasDirectoryType method to KDesktopFile. This is required for review request [119869](https://git.reviewboard.kde.org/r/119869/) Diffs - src/core/kdesktopfile.h 2190051 src/core/kdesktopfile.cpp 6278309 Diff: https://git.reviewboard.kde.org/r/119868/diff/ Testing --- Thanks, Maarten De Meyer
Re: Review Request 119868: KConfig add DirectoryType support
On Aug. 21, 2014, 7:19 a.m., David Faure wrote: We have never needed this until now because we can recognize such files by their name... So while I'm not opposed to this, I wonder, why not use if (u.fileName() == .directory) in KDesktopFileActions (RR 119869) -- which will also make it work with existing .directory files that got created without a Type=Directory entry? The simple solution is always the best one. Thanks! :) - Maarten --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119868/#review64952 --- On Aug. 21, 2014, 1:15 p.m., Maarten De Meyer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119868/ --- (Updated Aug. 21, 2014, 1:15 p.m.) Review request for KDE Frameworks, kdelibs and Frank Reininghaus. Repository: kconfig Description --- Add a hasDirectoryType method to KDesktopFile. This is required for review request [119869](https://git.reviewboard.kde.org/r/119869/) Diffs - src/core/kdesktopfile.h 2190051 src/core/kdesktopfile.cpp 6278309 Diff: https://git.reviewboard.kde.org/r/119868/diff/ Testing --- Thanks, Maarten De Meyer
Re: Review Request 119869: KIO: Correctly handle .directory files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119869/ --- (Updated Aug. 20, 2014, 10:35 p.m.) Review request for KDE Frameworks, kdelibs and Frank Reininghaus. Bugs: 235457 https://bugs.kde.org/show_bug.cgi?id=235457 Repository: kio Description --- My attempt at fixing [bug 235457](https://bugs.kde.org/show_bug.cgi?id=235457) We give .directory the default Type 'Directory' and a way for KDesktopFileActions to handle .directory files. To do that we trick KRun in opening the file as plain text. hasDirectoryType is a new method in KConfig. see: [119868](https://git.reviewboard.kde.org/r/119868/) Anything I should look out for when commiting this to KConfig and KIO? Diffs - src/widgets/kdesktopfileactions.cpp 9486015 src/widgets/kpropertiesdialog.cpp 346cbe8 Diff: https://git.reviewboard.kde.org/r/119869/diff/ Testing --- Configure a folder with a custom icon in dolphin. Click the created .directory file. Thanks, Maarten De Meyer
Re: Review Request 119868: KConfig add DirectoryType support
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119868/ --- (Updated Aug. 20, 2014, 10:34 p.m.) Review request for KDE Frameworks, kdelibs and Frank Reininghaus. Repository: kconfig Description --- Add a hasDirectoryType method to KDesktopFile. This is required for review request [119869](https://git.reviewboard.kde.org/r/119869/) Diffs - src/core/kdesktopfile.h 2190051 src/core/kdesktopfile.cpp 6278309 Diff: https://git.reviewboard.kde.org/r/119868/diff/ Testing --- Thanks, Maarten De Meyer
Re: Review Request 113607: Install all user agent desktop files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/113607/ --- (Updated Jan. 24, 2014, 12:06 p.m.) Status -- This change has been marked as submitted. Review request for KDE Base Apps. Repository: kde-baseapps Description --- Currently each user agent file has to be listed separately in the CMakeLists.txt file. The latest commit that added new files forgot to list them so they are never installed. Fixed properly by GLOBING for *.desktop Diffs - konqueror/settings/kio/uasproviders/CMakeLists.txt 6c49f42 Diff: https://git.reviewboard.kde.org/r/113607/diff/ Testing --- Builds. Nobody gets left behind! Thanks, Maarten De Meyer
Re: Review Request 110388: Change default thumbnail cache directory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110388/ --- (Updated Jan. 11, 2014, 3:15 p.m.) Status -- This change has been discarded. Review request for kdelibs. Bugs: 319528 http://bugs.kde.org/show_bug.cgi?id=319528 Repository: kdelibs Description --- The freedesktop spec[0] has changed the default location of the thumbnail cache directory. Now thumbnails are saved in $XDG_CACHE_HOME/thumbnails instead of ~/.thumbnails/ GNOME has already made this change. Some applications[1] and documentation[2] (maybe thumbnailers) will have to be changed to adapt to this new directory. If this patch is accepted I'll make the needed changes. For frameworks we can use QStandardPaths::CacheLocation, I'll post a separate review for that. All thumbnails will have to be regenerated. We could symlink the directory? Is fromLocal8Bit correct or is UTF8 needed now? [0] http://specifications.freedesktop.org/thumbnail-spec/thumbnail-spec-latest.html [1] configurepreviewdialog.cpp in Dolphin [2]http://techbase.kde.org/KDE_System_Administration/XDG_Filesystem_Hierarchy#Thumbnails Diffs - kio/kio/previewjob.cpp 01b674d Diff: https://git.reviewboard.kde.org/r/110388/diff/ Testing --- Remove ~/.thumbnails/ Run dolphin with previews: thumbnails are created in .cache/thumbnails/ Repeated with XDG_CACHE_HOME=~/.cachetestdir/ Thumbnails in .cachetestdir/thumbnails/ Not tested: gwenview and digikam. Thanks, Maarten De Meyer
Re: Review Request 113607: Install all user agent desktop files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/113607/#review47194 --- Bump? This is a very trivial change. Should I just commit this kind of stuff in the future instead of letting it rot for weeks? - Maarten De Meyer On Nov. 4, 2013, 5:19 p.m., Maarten De Meyer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/113607/ --- (Updated Nov. 4, 2013, 5:19 p.m.) Review request for KDE Base Apps. Repository: kde-baseapps Description --- Currently each user agent file has to be listed separately in the CMakeLists.txt file. The latest commit that added new files forgot to list them so they are never installed. Fixed properly by GLOBING for *.desktop Diffs - konqueror/settings/kio/uasproviders/CMakeLists.txt 6c49f42 Diff: https://git.reviewboard.kde.org/r/113607/diff/ Testing --- Builds. Nobody gets left behind! Thanks, Maarten De Meyer
Review Request 113607: Install all user agent desktop files
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113607/ --- Review request for KDE Base Apps. Repository: kde-baseapps Description --- Currently each user agent file has to be listed separately in the CMakeLists.txt file. The latest commit that added new files forgot to list them so they are never installed. Fixed properly by GLOBING for *.desktop Diffs - konqueror/settings/kio/uasproviders/CMakeLists.txt 6c49f42 Diff: http://git.reviewboard.kde.org/r/113607/diff/ Testing --- Builds. Nobody gets left behind! Thanks, Maarten De Meyer
Review Request 113608: Add modern user agents for common browsers
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113608/ --- Review request for KDE Base Apps. Repository: kde-baseapps Description --- I recently needed to swith user agents to (very) new browser versions. This is an attempt to add them upstream. Since browser makers release new version every other week it seems useless to make a new file for each one. I just named it latest and don't include the version number in the description. That way translators have no extra work when we update the user agent. Diffs - konqueror/settings/kio/uasproviders/android_latest.desktop PRE-CREATION konqueror/settings/kio/uasproviders/chrome_latest.desktop PRE-CREATION konqueror/settings/kio/uasproviders/firefox_latest.desktop PRE-CREATION konqueror/settings/kio/uasproviders/ie_latest.desktop PRE-CREATION Diff: http://git.reviewboard.kde.org/r/113608/diff/ Testing --- Thanks, Maarten De Meyer
Re: Review Request 110423: Searchprovider dialog: add insert query placeholder button
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110423/ --- (Updated May 28, 2013, 6:37 p.m.) Review request for KDE Runtime. Changes --- Ping? I added an alternative look of the button (icon only) that we could use. Description --- This adds a 'Insert query placeholder' button to the add/modify web shortcut dialog. -Clicking this button inserts \{@} in the shortcut URL lineEdit. -The button is only enabled when the shortcut URL lineEdit has focus. This makes it easy to add new web shortcuts. Users don't have to remember and type the query placeholder. The eventfilter complicates this patch a lot. But there is no slot to detect if a lineedit has focus. We could drop it but it looks messy when the button is always enabled. I feel the tooltip text can be improved. Any suggestions from a native English speaker? This addresses bug 146879. http://bugs.kde.org/show_bug.cgi?id=146879 Diffs - kurifilter-plugins/ikws/searchproviderdlg.h e931e11 kurifilter-plugins/ikws/searchproviderdlg.cpp 5f40f5f kurifilter-plugins/ikws/searchproviderdlg_ui.ui d75ac5b Diff: http://git.reviewboard.kde.org/r/110423/diff/ Testing --- Added a new shortcut. Changed focus to and from different widgets. Inserted placeholder at the end of the link and in the middle. Modified a shortcut. File Attachments (updated) Berfore vs After http://git.reviewboard.kde.org/media/uploaded/files/2013/05/21/before-After2.png Alternative look http://git.reviewboard.kde.org/media/uploaded/files/2013/05/28/After3.png Thanks, Maarten De Meyer
Re: Review Request 110423: Searchprovider dialog: add insert query placeholder button
On May 17, 2013, 10:36 p.m., Dawit Alemayehu wrote: kurifilter-plugins/ikws/searchproviderdlg.cpp, line 185 http://git.reviewboard.kde.org/r/110423/diff/1/?file=143674#file143674line185 Why not simply disable the button on a FocusOut event for m_dlg.leQuery ? That way you do not need to install an event filter on all the edits. Because the focusout event is called before the button signal is emitted. So when clicking the button it gets disabled and does nothing :) But I dropped the eventfilter code in the new version - Maarten --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110423/#review32716 --- On May 21, 2013, 3:22 p.m., Maarten De Meyer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110423/ --- (Updated May 21, 2013, 3:22 p.m.) Review request for KDE Runtime. Description --- This adds a 'Insert query placeholder' button to the add/modify web shortcut dialog. -Clicking this button inserts \{@} in the shortcut URL lineEdit. -The button is only enabled when the shortcut URL lineEdit has focus. This makes it easy to add new web shortcuts. Users don't have to remember and type the query placeholder. The eventfilter complicates this patch a lot. But there is no slot to detect if a lineedit has focus. We could drop it but it looks messy when the button is always enabled. I feel the tooltip text can be improved. Any suggestions from a native English speaker? This addresses bug 146879. http://bugs.kde.org/show_bug.cgi?id=146879 Diffs - kurifilter-plugins/ikws/searchproviderdlg.h e931e11 kurifilter-plugins/ikws/searchproviderdlg.cpp 5f40f5f kurifilter-plugins/ikws/searchproviderdlg_ui.ui d75ac5b Diff: http://git.reviewboard.kde.org/r/110423/diff/ Testing --- Added a new shortcut. Changed focus to and from different widgets. Inserted placeholder at the end of the link and in the middle. Modified a shortcut. File Attachments Before - After screenshot http://git.reviewboard.kde.org/media/uploaded/files/2013/05/14/before-After.png Berfore vs After http://git.reviewboard.kde.org/media/uploaded/files/2013/05/21/before-After2.png Thanks, Maarten De Meyer
Review Request 110423: Searchprovider dialog: add insert query placeholder button
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110423/ --- Review request for KDE Runtime. Description --- This adds a 'Insert query placeholder' button to the add/modify web shortcut dialog. -Clicking this button inserts \{@} in the shortcut URL lineEdit. -The button is only enabled when the shortcut URL lineEdit has focus. This makes it easy to add new web shortcuts. Users don't have to remember and type the query placeholder. The eventfilter complicates this patch a lot. But there is no slot to detect if a lineedit has focus. We could drop it but it looks messy when the button is always enabled. This addresses bug 146879. http://bugs.kde.org/show_bug.cgi?id=146879 Diffs - kurifilter-plugins/ikws/searchproviderdlg.h e931e11 kurifilter-plugins/ikws/searchproviderdlg.cpp 5f40f5f Diff: http://git.reviewboard.kde.org/r/110423/diff/ Testing --- Added a new shortcut. Changed focus to and from different widgets. Inserted placeholder at the end of the link and in the middle. Modified a shortcut. File Attachments Before - After screenshot http://git.reviewboard.kde.org/media/uploaded/files/2013/05/14/before-After.png Thanks, Maarten De Meyer
Re: Review Request 110423: Searchprovider dialog: add insert query placeholder button
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110423/ --- (Updated May 14, 2013, 1:52 p.m.) Review request for KDE Runtime. Description (updated) --- This adds a 'Insert query placeholder' button to the add/modify web shortcut dialog. -Clicking this button inserts \{@} in the shortcut URL lineEdit. -The button is only enabled when the shortcut URL lineEdit has focus. This makes it easy to add new web shortcuts. Users don't have to remember and type the query placeholder. The eventfilter complicates this patch a lot. But there is no slot to detect if a lineedit has focus. We could drop it but it looks messy when the button is always enabled. I feel the looltip text can be improved. Any suggestions from a native English speaker? This addresses bug 146879. http://bugs.kde.org/show_bug.cgi?id=146879 Diffs - kurifilter-plugins/ikws/searchproviderdlg.h e931e11 kurifilter-plugins/ikws/searchproviderdlg.cpp 5f40f5f Diff: http://git.reviewboard.kde.org/r/110423/diff/ Testing --- Added a new shortcut. Changed focus to and from different widgets. Inserted placeholder at the end of the link and in the middle. Modified a shortcut. File Attachments Before - After screenshot http://git.reviewboard.kde.org/media/uploaded/files/2013/05/14/before-After.png Thanks, Maarten De Meyer
Re: Review Request 110423: Searchprovider dialog: add insert query placeholder button
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110423/ --- (Updated May 14, 2013, 1:53 p.m.) Review request for KDE Runtime. Changes --- darn typo's! Description (updated) --- This adds a 'Insert query placeholder' button to the add/modify web shortcut dialog. -Clicking this button inserts \{@} in the shortcut URL lineEdit. -The button is only enabled when the shortcut URL lineEdit has focus. This makes it easy to add new web shortcuts. Users don't have to remember and type the query placeholder. The eventfilter complicates this patch a lot. But there is no slot to detect if a lineedit has focus. We could drop it but it looks messy when the button is always enabled. I feel the tooltip text can be improved. Any suggestions from a native English speaker? This addresses bug 146879. http://bugs.kde.org/show_bug.cgi?id=146879 Diffs - kurifilter-plugins/ikws/searchproviderdlg.h e931e11 kurifilter-plugins/ikws/searchproviderdlg.cpp 5f40f5f Diff: http://git.reviewboard.kde.org/r/110423/diff/ Testing --- Added a new shortcut. Changed focus to and from different widgets. Inserted placeholder at the end of the link and in the middle. Modified a shortcut. File Attachments Before - After screenshot http://git.reviewboard.kde.org/media/uploaded/files/2013/05/14/before-After.png Thanks, Maarten De Meyer
Review Request 110388: Change default thumbnail cache directory
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110388/ --- Review request for kdelibs. Description --- The freedesktop spec[0] has changed the default location of the thumbnail cache directory. Now thumbnails are saved in $XDG_CACHE_HOME/thumbnails instead of ~/.thumbnails/ GNOME has already made this change. Some applications[1] and documentation[2] (maybe thumbnailers) will have to be changed to adapt to this new directory. If this patch is accepted I'll make the needed changes. For frameworks we can use QStandardPaths::CacheLocation, I'll post a separate review for that. All thumbnails will have to be regenerated. We could symlink the directory? Is fromLocal8Bit correct or is UTF8 needed now? [0] http://specifications.freedesktop.org/thumbnail-spec/thumbnail-spec-latest.html [1] configurepreviewdialog.cpp in Dolphin [2]http://techbase.kde.org/KDE_System_Administration/XDG_Filesystem_Hierarchy#Thumbnails This addresses bug 319528. http://bugs.kde.org/show_bug.cgi?id=319528 Diffs - kio/kio/previewjob.cpp 01b674d Diff: http://git.reviewboard.kde.org/r/110388/diff/ Testing --- Remove ~/.thumbnails/ Run dolphin with previews: thumbnails are created in .cache/thumbnails/ Repeated with XDG_CACHE_HOME=~/.cachetestdir/ Thumbnails in .cachetestdir/thumbnails/ Not tested: gwenview and digikam. Thanks, Maarten De Meyer
Review Request 110277: Fix broken web shortcuts
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110277/ --- Review request for KDE Runtime. Description --- I have gone through and and tested most (if not all) of the default web shortcuts in kde-runtime. Changes: http://lists.kde.org/?l=kde-core-develm=136750251617156w=2 Diffs - kurifilter-plugins/ikws/searchproviders/cia.desktop d39c38b kurifilter-plugins/ikws/searchproviders/backports.desktop 8ebdd67 kurifilter-plugins/ikws/searchproviders/austronaut.desktop b6a5fe7 kurifilter-plugins/ikws/searchproviders/amg.desktop ad9f70a kurifilter-plugins/ikws/searchproviders/altavista.desktop 4620f92 kurifilter-plugins/ikws/searchproviders/acronym.desktop c93ad96 kurifilter-plugins/ikws/searchproviders/citeseer.desktop 8158f03 kurifilter-plugins/ikws/searchproviders/ctan.desktop 4a2807d kurifilter-plugins/ikws/searchproviders/feedster.desktop 13ff3d7 kurifilter-plugins/ikws/searchproviders/foldoc.desktop 3a7608c kurifilter-plugins/ikws/searchproviders/freecode.desktop PRE-CREATION kurifilter-plugins/ikws/searchproviders/freshmeat.desktop 5e84ffa kurifilter-plugins/ikws/searchproviders/froogle.desktop 75318aa kurifilter-plugins/ikws/searchproviders/google_images.desktop 3c7a7b0 kurifilter-plugins/ikws/searchproviders/google_movie.desktop 5755c3a kurifilter-plugins/ikws/searchproviders/google_shopping.desktop PRE-CREATION kurifilter-plugins/ikws/searchproviders/gracenote.desktop b96a51b kurifilter-plugins/ikws/searchproviders/jamendo.desktop 8d971bd kurifilter-plugins/ikws/searchproviders/katatudo.desktop 352b7b3 kurifilter-plugins/ikws/searchproviders/kde_projects.desktop PRE-CREATION kurifilter-plugins/ikws/searchproviders/kde_websvn.desktop 1310fb0 kurifilter-plugins/ikws/searchproviders/metacrawler.desktop 8d07aa7 kurifilter-plugins/ikws/searchproviders/msdn.desktop 4ee9823 kurifilter-plugins/ikws/searchproviders/nl-telephone.desktop 3827d71 kurifilter-plugins/ikws/searchproviders/pgpkeys.desktop 1476496 kurifilter-plugins/ikws/searchproviders/python.desktop 9542815 kurifilter-plugins/ikws/searchproviders/qt.desktop df6b25f kurifilter-plugins/ikws/searchproviders/qt3.desktop 5204151 kurifilter-plugins/ikws/searchproviders/qt4.desktop PRE-CREATION kurifilter-plugins/ikws/searchproviders/rfc.desktop 1ce3648 kurifilter-plugins/ikws/searchproviders/technorati.desktop a10a8d7 kurifilter-plugins/ikws/searchproviders/vimeo.desktop d32ab7e kurifilter-plugins/ikws/searchproviders/vivisimo.desktop 291150e kurifilter-plugins/tests/kurifiltertest.cpp dd18eba Diff: http://git.reviewboard.kde.org/r/110277/diff/ Testing --- Tried every search at least once. Thanks, Maarten De Meyer
Re: Review Request 110313: Some KUrifilter-plugin Krazy fixes
On May 5, 2013, 4:09 a.m., Dawit Alemayehu wrote: kurifilter-plugins/ikws/ikwsopts.cpp, line 107 http://git.reviewboard.kde.org/r/110313/diff/2/?file=142219#file142219line107 what exactly is nl/ ? Is this i18n specific keyword ? Yes it is il8n semantic markup. nl/ is a Line break, perhaps para tags are better here? http://techbase.kde.org/Development/Tutorials/Localization/i18n_Semantics#Semantic_Tags On May 5, 2013, 4:09 a.m., Dawit Alemayehu wrote: kurifilter-plugins/ikws/ikwsopts.cpp, line 412 http://git.reviewboard.kde.org/r/110313/diff/2/?file=142219#file142219line412 What is the point of using a QPointer when the parent is set to NULL ? The parent is not NULL, it is this (third argument). The firs argument is the provider to be edited, or when creating a new provider: 0 Please correct me if I'm wrong. - Maarten --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110313/#review32045 --- On May 4, 2013, 10:50 p.m., Maarten De Meyer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110313/ --- (Updated May 4, 2013, 10:50 p.m.) Review request for KDE Runtime. Description --- This fixes most of the Krazy fixes for KUrifilter-plugins. http://www.englishbreakfastnetwork.org/krazy/reports/kde-4.x/kde-runtime/kurifilter-plugins/index.html Diffs - kurifilter-plugins/shorturi/kshorturifilter.cpp 802b325 kurifilter-plugins/localdomain/localdomainurifilter.h 894b624 kurifilter-plugins/ikws/searchproviderdlg.cpp eb4b31d kurifilter-plugins/ikws/searchprovider.h 7f26ba7 kurifilter-plugins/ikws/ikwsopts_ui.ui 42e9675 kurifilter-plugins/ikws/ikwsopts_p.h d048f08 kurifilter-plugins/ikws/ikwsopts.cpp 8ca80b6 kurifilter-plugins/fixhost/fixhosturifilter.h 8603e36 kurifilter-plugins/tests/kurifiltertest.cpp dd18eba Diff: http://git.reviewboard.kde.org/r/110313/diff/ Testing --- Compiles. Only 4 issues remaining. Thanks, Maarten De Meyer
Re: Review Request 110247: New web shortcut. Copy link from clipboard
On May 2, 2013, 9:30 a.m., David Faure wrote: kurifilter-plugins/ikws/searchproviderdlg.cpp, line 68 http://git.reviewboard.kde.org/r/110247/diff/1/?file=141528#file141528line68 The argument is not necessary, Clipboard is the default value. Fixed both problems locally. Thank you for the review :) - Maarten --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110247/#review31875 --- On May 2, 2013, 11:53 a.m., Maarten De Meyer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110247/ --- (Updated May 2, 2013, 11:53 a.m.) Review request for KDE Runtime. Description --- When making a new web shortcut check the clipboard, if it's a url put it in the query line edit. Just a little time saver. (akregator does something similar) I used !host().isEmpty() because isValid() is almost always valid. This will give false positives where the wrong link is placed inside the query box. This addresses bug 146880. http://bugs.kde.org/show_bug.cgi?id=146880 Diffs - kurifilter-plugins/ikws/searchproviderdlg.cpp 2bb2852 Diff: http://git.reviewboard.kde.org/r/110247/diff/ Testing --- Tested some url's and some random text. Only a url gets selected from the clipboard. Thanks, Maarten De Meyer
Proposed web shortcut changes
Hi everybody, I have gone through and and tested most (if not all) of the default web shortcuts in kde-runtime. These are my proposed changes and notes to fix the broken ones. Warning: long mail! 1. Most of the shortcuts that were no longer working only needed an updated url. Amg: http://www.allmusic.com/search/all/\\{@} backports: http://packages.debian.org/search?suite=squeeze-backportssection=allarch=anysearchon=nameskeywords=\\{@} note: This is rather distro specific so maybe this should be included downstream? CIA: https://www.cia.gov/search?q=\\{@}site=WORLD_FACTBOOKbtnG=Searchclient=CIAmyAction=/searchproxystylesheet=CIAsubmitMethod=getoe=UTF-8ie=UTF-8ud=1 citeseer: http://citeseer.ist.psu.edu/search?q=\\{@}submit=Searchsort=rlvt=doc CTAN Catalog: http://ctan.org/search?phrase=\\{@} Feedster: http://www.feedster.com/index.php?page=search/webtype=Websearch=\\{@} foldoc: http://foldoc.org/\\{@} Google images: https://www.google.com/search?site=imghptbm=ischq=\\{@} note: Not broken but now uses https. Google movies: http://www.google.com/search?q=\\{@}ie=UTF-8oe=UTF-8 jamendo: http://www.jamendo.com/en/search?qs=q=test metacrawler: http://www.metacrawler.com/info.metac.cloud.other/search/web?q=\\{@} msdn: https://social.msdn.microsoft.com/Search/en-US?query=\\{@} nl-telephone: http://www.detelefoongids.nl/\\{1}/\\{2}/10-1/?oWhat=\\{1}oWhere=\\{2} note: 1 = name, 2 = loacation Why is this only for the netherlands? Perhaps make this into a more international service. With the localised query's using different sites. python: https://www.google.com/search?sourceid=python-searchq=\\{@}submit=Searchq=site%3Apython.org note: Not broken, changed to https Qt: http://qt-project.org/doc/search/qt-5.0?search=\\{@} Do we need an extra qt4 documentation shortcut? (Qt3 is removed) rfc: http://tools.ietf.org/id/\\{@} technorati: http://technorati.com/search?return=sitesauthority=highq=\\{@} Vimeo: http://vimeo.com/search?q=\\{@} 2. These are the shortcuts that have moved to a new website. Instead or renaming it will be easier to remove the old ones and add the new site. acronym The chemical acronyms on http://www.chemeurope.com/en/tools/ are no longer accessible using the url. A good alternative is http://www.abbreviations.com/\\{@} It offers acronyms for chemical elements and lots of other words. Disadvantages: Lots of in-your-face adds, not translabable from the url freshmeat Freshmeat has been renamed freecode. http://freecode.com/search?q=\\{@} froogle Froogle has been renamed google product search and renamed again to google shopping. :) Removed froogle.desktop and added google shopping. https://www.google.com/search?hl=entbm=shopq=\\{@} KDE WebSVN Svn usage in KDE is on it's way out, maybe we can reflect that in the web shortcuts. I propose to remove websvn and replace it with the KDE Projects site. https://projects.kde.org/search?projects=1q=rekonq note: websvn is a bit hard to use but not broken, we could keep it AND include KDEprojects. OpenPGP Key Search The origianal domain was not working, changed to pgp.mit.edu. There are others available, not sure on which one is best. http://pgp.mit.edu:11371/pks/lookup?search=\\{@}op=vindex 3. Some of the shortcuts lead to websites that are no longer active, are sold or have no working search function. These should be removed. Altavista The altavista search has been replased by yahoo. It's just yahoo with an altavista logo. We could update the query but I propose to remove it. Austronaut The website is for sale. http://austronaut.at/ Gracenote I cannot find the web search, only an api is available. KataTudo Both the website and the search function are broken. http://www.katatudo.com.br/ opendesktop kde-look kde-apps The search is not working correctly and cannot be changed because it is handled using POST. I have contacted Frank Karlitschek to see if this might be fixed server side. If not these will remain broken and should be removed imo. Qt3 Qt5 was released recently, it's time to let go :) vivisimo sold to IBM. Thank you if you read this far, please share any feedback you have. Questions: If people agree with the proposed changes is it ok to put it in one big patch on reviewboard? I first wanted to get some comments using the mailing list. Some shortcuts offer revenue sharing with KDE. Can a developer not associated with KDE ev. make these 'deals'? I think amazon offers something like that.. Thank you Maarten De Meyer
Review Request 110259: Fix kurifiltertest to use the new google query
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110259/ --- Review request for KDE Runtime. Description --- My previous review (https://git.reviewboard.kde.org/r/109841/) broke kurifiltertest. The google web shortcut has been changed to use https. This made the kurifilter unit test fail some tests. Changed the unit test to check for https. Diffs - kurifilter-plugins/tests/kurifiltertest.cpp 4677a91 Diff: http://git.reviewboard.kde.org/r/110259/diff/ Testing --- All tests pass. Thanks, Maarten De Meyer
Review Request 110247: New web shortcut. Copy link from clipboard
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110247/ --- Review request for KDE Runtime. Description --- When making a new web shortcut check the clipboard, if it's a url put it in the query line edit. Just a little time saver. (akregator does something similar) I used !host().isEmpty() because isValid() is almost always valid. This will give false positives where the wrong link is placed inside the query box. This addresses bug 146880. http://bugs.kde.org/show_bug.cgi?id=146880 Diffs - kurifilter-plugins/ikws/searchproviderdlg.cpp 2bb2852 Diff: http://git.reviewboard.kde.org/r/110247/diff/ Testing --- Tested some url's and some random text. Only a url gets selected from the clipboard. Thanks, Maarten De Meyer
Re: Review Request 109841: Use https for web shortcuts
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109841/ --- (Updated April 22, 2013, 7:10 p.m.) Review request for KDE Runtime. Changes --- Do not change translations. Description --- Use encrypted https for the web shortcuts that support it. Make sure the site does not redirect to unencrypted connection. (exception: google feeling lucky) Other shortcuts need more changes to support SSL. A lot are outdated, I'll post a patch for them soonish. This small change should improve protection against networks sniffers. This addresses bug 308029. http://bugs.kde.org/show_bug.cgi?id=308029 Diffs (updated) - kurifilter-plugins/ikws/searchproviders/blip.desktop b308ad3 kurifilter-plugins/ikws/searchproviders/bugft.desktop f7aeff7 kurifilter-plugins/ikws/searchproviders/duckduckgo.desktop 0ac02ed kurifilter-plugins/ikws/searchproviders/duckduckgo_info.desktop 69dbabb kurifilter-plugins/ikws/searchproviders/duckduckgo_shopping.desktop 538945f kurifilter-plugins/ikws/searchproviders/facebook.desktop 1ab6de1 kurifilter-plugins/ikws/searchproviders/flickrcc.desktop 8ab0044 kurifilter-plugins/ikws/searchproviders/github.desktop 5af6fe6 kurifilter-plugins/ikws/searchproviders/gitorious.desktop 2a801d9 kurifilter-plugins/ikws/searchproviders/google.desktop 86fc2e3 kurifilter-plugins/ikws/searchproviders/google_advanced.desktop e3073b9 kurifilter-plugins/ikws/searchproviders/google_code.desktop 3a6f94f kurifilter-plugins/ikws/searchproviders/google_groups.desktop 0aa3e7e kurifilter-plugins/ikws/searchproviders/google_lucky.desktop 8b9837f kurifilter-plugins/ikws/searchproviders/google_maps.desktop 2beef40 kurifilter-plugins/ikws/searchproviders/google_news.desktop beccf05 kurifilter-plugins/ikws/searchproviders/identica_groups.desktop c978be9 kurifilter-plugins/ikws/searchproviders/identica_notices.desktop 6c723d4 kurifilter-plugins/ikws/searchproviders/identica_people.desktop 41b0064 kurifilter-plugins/ikws/searchproviders/kde_forums.desktop e7acaaa kurifilter-plugins/ikws/searchproviders/wikipedia.desktop 02a3241 kurifilter-plugins/ikws/searchproviders/wiktionary.desktop 1655241 kurifilter-plugins/ikws/searchproviders/wolfram_alpha.desktop 6a8bda6 kurifilter-plugins/ikws/searchproviders/youtube.desktop 0549216 Diff: http://git.reviewboard.kde.org/r/109841/diff/ Testing --- All shortcuts use https and have a valid ssl certificate. Thanks, Maarten De Meyer
Review Request 109841: Use https for web shortcuts
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109841/ --- Review request for KDE Runtime. Description --- Use encrypted https for the web shortcuts that support it. Make sure the site does not redirect to unencrypted connection. (exception: google feeling lucky) Other shortcuts need more changes to support SSL. A lot are outdated, I'll post a patch for them soonish. This small change should improve protection against networks sniffers. This addresses bug 308029. http://bugs.kde.org/show_bug.cgi?id=308029 Diffs - kurifilter-plugins/ikws/searchproviders/blip.desktop b308ad3 kurifilter-plugins/ikws/searchproviders/facebook.desktop 1ab6de1 kurifilter-plugins/ikws/searchproviders/flickrcc.desktop 8ab0044 kurifilter-plugins/ikws/searchproviders/github.desktop 5af6fe6 kurifilter-plugins/ikws/searchproviders/gitorious.desktop 2a801d9 kurifilter-plugins/ikws/searchproviders/google.desktop 86fc2e3 kurifilter-plugins/ikws/searchproviders/google_advanced.desktop 5ec1bba kurifilter-plugins/ikws/searchproviders/google_groups.desktop 7366c90 kurifilter-plugins/ikws/searchproviders/google_lucky.desktop 8b9837f kurifilter-plugins/ikws/searchproviders/google_news.desktop 1ef636c kurifilter-plugins/ikws/searchproviders/identica_groups.desktop 4d3efe8 kurifilter-plugins/ikws/searchproviders/identica_notices.desktop 2ed0e5f kurifilter-plugins/ikws/searchproviders/identica_people.desktop d233cb8 kurifilter-plugins/ikws/searchproviders/kde_forums.desktop e7acaaa kurifilter-plugins/ikws/searchproviders/wolfram_alpha.desktop 6a8bda6 kurifilter-plugins/ikws/searchproviders/youtube.desktop 0549216 Diff: http://git.reviewboard.kde.org/r/109841/diff/ Testing --- All shortcuts use https and have a valid ssl certificate. Thanks, Maarten De Meyer
Re: Review Request: Rename Samba Shares to Windows Shares (SMB)
On June 21, 2012, 10:05 p.m., Mark Gaiser wrote: I don't really know if we should do this.. I do understand why you want to rename it. From a user point of view the user probably wants to either access windows shares or make shares accessible for windows. Either way, it's done through Samba which implements the SMB protocol: http://en.wikipedia.org/wiki/Server_Message_Block so it are actually SMB Shares ;) Changing it is not really useful i think. I for example make Samba shares between Linux machines as well since that is just easy and why should those be called Windows Shares all of a sudden? I think Samba Shares is just fine. Perhaps SMB Shares describes it even better. I'm not against the change, but also not in favor. Someone else should decide whether this is OK or not. Lamarque Vieira Souza wrote: I am also against this change, also because Windows is a trademark, so we have better avoid using it in KDE. OBS: samba also implements the cifs protocol, the sucessor of SMB and the current default protocol used in Win7. So SMB Shares is also not a good name. Andrius da Costa Ribas wrote: On Windows a \\ path is translated into a smb:// URL for KDE apps, so no Samba is used at all but the bookmark is still called Samba Shares I know it uses the SMB protocol, thats why I added it in brackets. We know samba. My mother however thinks about a certain latin dance when I tell her to 'use the samba share'. I don't know what it is called in other linux DE's and it doesn't matter, we should chose the best fitting string. There are different interpretations of best. For me that means it hides implementation details and is easy to understand for everybody. Other people might want to call it what it is. SMB would be even worse compared to samba imo, this change is to make it easier for new users, not harder. Not sure about the trademark, I doubt it would be a problem but am obviously no lawyer. This is not really an important change, if this is considered bad I'll just close this review and the bug. It was more a personal attempt to get used to bug fixing workflow. - Maarten --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105318/#review14969 --- On June 21, 2012, 3:40 p.m., Maarten De Meyer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105318/ --- (Updated June 21, 2012, 3:40 p.m.) Review request for KDE Runtime and David Edmundson. Description --- Fix for bug 141257. Renamed 'Samba Shares' to 'Windows Shares (SMB)' I posted this on the usability mailing list a while a go and David Edmundson CC'd the original author but for now there has been no response. http://lists.kde.org/?l=kde-usabilitym=133173657001169w=2 Since we are past string freeze this is probably for 4.10 I can not commit this myself, thanks This addresses bug 141257. http://bugs.kde.org/show_bug.cgi?id=141257 Diffs - kioslave/smb/smb-network.desktop a121a31 Diff: http://git.reviewboard.kde.org/r/105318/diff/ Testing --- Thanks, Maarten De Meyer
Review Request: Rename Samba Shares to Windows Shares (SMB)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105318/ --- Review request for KDE Runtime and David Edmundson. Description --- Fix for bug 141257. Renamed 'Samba Shares' to 'Windows Shares (SMB)' I posted this on the usability mailing list a while a go and David Edmundson CC'd the original author but for now there has been no response. http://lists.kde.org/?l=kde-usabilitym=133173657001169w=2 Since we are past string freeze this is probably for 4.10 I can not commit this myself, thanks This addresses bug 141257. http://bugs.kde.org/show_bug.cgi?id=141257 Diffs - kioslave/smb/smb-network.desktop a121a31 Diff: http://git.reviewboard.kde.org/r/105318/diff/ Testing --- Thanks, Maarten De Meyer
Review Request: UDisks mount vfat filesystems with flush option.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105224/ --- Review request for kdelibs, Solid and Lukáš Tinkl. Description --- This patch mounts vfat filesystems (mostly usb devices) with the flush option when using the UDisks backend. The hal backend already did this. The flush option makes sure changes are written to the device immediately. Please let me know it this should be implemented in an other (optional) way or if this should be in the UDev rules directly. ps. I do not have commit rights. This addresses bug 273792. http://bugs.kde.org/show_bug.cgi?id=273792 Diffs - solid/solid/backends/udisks/udisksstorageaccess.cpp 7d72a88 Diff: http://git.reviewboard.kde.org/r/105224/diff/ Testing --- Compiled without error, mounted multiple devices without problems. Thanks, Maarten De Meyer
Re: Review Request: Change Online help icon KHelpcenter
On April 17, 2012, 1:45 p.m., Sebastian Kügler wrote: This change is wrong, as the menu entry has nothing to do with the semantic meaning of the icon, and the icon is not named according to the icon spec. So the correct icon is already set here, if its look doesn't match, then that icon would need to be fixed. In this case, I assume you mean to better reflect the online part in the name, and I agree that it's not reflected in the name. Question is: does it matter where the help is located? (Surely does if the user is offline, but in general ... I think the help! part is important, not the online part. Albert Astals Cid wrote: +1 The openSuse icons had me confused. The one I mean is 'applications-internet' The current icon is the same as most other entries: a book with a question mark. Not really wrong, but not correct/obvious either. I do however think that the 'online' part is more important. If you are in the help center EVERYTHING is 'help', it should be more obvious what the difference is between the available options to get help. And these days more and more help is located online and no longer limited to only documentation. (think forums, wiki, ...) I wish for the help center to be more of place to go when you have problems than a handy manual reader. (In all honesty changing a little icon won't make much difference) As this is my first patch I'm not sure what the policy is to do now. Close as discarded or leave open? - Maarten --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104621/#review12573 --- On April 16, 2012, 5:28 p.m., Maarten De Meyer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104621/ --- (Updated April 16, 2012, 5:28 p.m.) Review request for KDE Runtime and Cornelius Schumacher. Description --- Changes the 'Online help' icon in the navigation to a more fitting one.(imho) Diffs - khelpcenter/plugins/onlinehelp.desktop 540f83f Diff: http://git.reviewboard.kde.org/r/104621/diff/ Testing --- compiled and run, works fine Thanks, Maarten De Meyer
Review Request: Fix Bug 152156 - KDE help center does not respect window placement policy
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104662/ --- Review request for KDE Runtime and Cornelius Schumacher. Description --- Fix Bug 152156 - KDE help center does not respect window placement policy khelpcenter does not open in the middle of the screen with the kwin centered window placement policy. 1) remove setGeometry on mainwindow 2) include defaultsize to setupGui (application looked strange when only using size hinting) Any suggestions on a better default? This addresses bug 152156. http://bugs.kde.org/show_bug.cgi?id=152156 Diffs - khelpcenter/mainwindow.cpp dd22425 Diff: http://git.reviewboard.kde.org/r/104662/diff/ Testing --- Compiled, run, changed size, run again, remove configuration, run again Thanks, Maarten De Meyer
Review Request: Change Online help icon KHelpcenter
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104621/ --- Review request for KDE Runtime and Cornelius Schumacher. Summary (updated) - Change Online help icon KHelpcenter Description (updated) --- Changes the 'Online help' icon in the navigation to a more fitting one.(imho) Diffs - khelpcenter/plugins/onlinehelp.desktop 540f83f Diff: http://git.reviewboard.kde.org/r/104621/diff/ Testing (updated) --- compiled and run, works fine Thanks, Maarten De Meyer