Review Request: Config cleanup on removeActivity
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106097/ --- Review request for KDE Runtime and Plasma. Description --- Remove the related configuration entry (which holds the current desktop) when an activity is removed. Diffs - src/service/Activities.cpp e7c1a7bfd5415127b804a2af4bb0b920fb2118ac Diff: http://git.reviewboard.kde.org/r/106097/diff/ Testing --- Yes Thanks, makis marimpis
Re: Review Request: in KUrlCompletion do not percent encode to make it human readable
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106096/ --- (Updated Aug. 20, 2012, 1:51 p.m.) Review request for kdelibs, David Faure and Thiago Macieira. Description --- when using konqueror and typing man: it starts to list possible entries which the kio_man slave generates. However, konqueror displays percent encoded URLs, e.g. man:%281%29/ instead of man:(1)/, which is not human readable. Also, there is some inconsistency in what konqueror shows in its completion list. E.g. when typing man:mklos it shows man:mklost%2Bfound in the completion list, but when I select this entry, the URL in the address line edit is changed and displays as man:mklost+found Even worse: when I now again type man:mklos, I get 2 entries in the completion list man:mklost%2Bfound and man:mklost+found (the one coming from the completion, the other from the history) No matter which one I chose, the result in the address line edit is always the unencoded one, which - for a user - makes much more sense. This patch removes the calls to make the matches percent encoded. Why would I ever want to get a percent encoded string from a completer, which is about helping a human ? Diffs - kio/kio/kurlcompletion.cpp 269fdc1 Diff: http://git.reviewboard.kde.org/r/106096/diff/ Testing --- man:, fish:, local dir /tmp and checking the completion in konqueror when using a dir e.g. named some ö ä ü umlauts, some file with#anchor Thanks, Martin Koller
Re: Review Request: in KUrlCompletion do not percent encode to make it human readable
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106096/#review17756 --- some file with#anchor is this an HTML anchor, or a filename with a hash? The real test for your change is the case of completing a remote file (e.g. sftp, fish, etc.) that contains a '#' in the filename. If you're skipping percent encoding for the '#', it will be parsed as an anchor, by the user of that URL, i.e. the filename will be incomplete. If you hit such a bug, the solution is to use the KUrl API better (e.g. addPath and then prettyUrl), so that only the necessary characters get escaped, rather than all of them, indeed. - David Faure On Aug. 20, 2012, 1:51 p.m., Martin Koller wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106096/ --- (Updated Aug. 20, 2012, 1:51 p.m.) Review request for kdelibs, David Faure and Thiago Macieira. Description --- when using konqueror and typing man: it starts to list possible entries which the kio_man slave generates. However, konqueror displays percent encoded URLs, e.g. man:%281%29/ instead of man:(1)/, which is not human readable. Also, there is some inconsistency in what konqueror shows in its completion list. E.g. when typing man:mklos it shows man:mklost%2Bfound in the completion list, but when I select this entry, the URL in the address line edit is changed and displays as man:mklost+found Even worse: when I now again type man:mklos, I get 2 entries in the completion list man:mklost%2Bfound and man:mklost+found (the one coming from the completion, the other from the history) No matter which one I chose, the result in the address line edit is always the unencoded one, which - for a user - makes much more sense. This patch removes the calls to make the matches percent encoded. Why would I ever want to get a percent encoded string from a completer, which is about helping a human ? Diffs - kio/kio/kurlcompletion.cpp 269fdc1 Diff: http://git.reviewboard.kde.org/r/106096/diff/ Testing --- man:, fish:, local dir /tmp and checking the completion in konqueror when using a dir e.g. named some ö ä ü umlauts, some file with#anchor Thanks, Martin Koller
Re: Review Request: Focus goes to location bar when opening link in new tab in foreground
On Aug. 20, 2012, 8:51 a.m., David Faure wrote: Thanks for looking into this. To be honest, I don't like the timer. It penalizes fast users, and it feels like a workaround. Surely at some point konqueror knows whether it's going to open a url in the tab or not. That's the point in the code where we should decide where to put the focus (and in terms of timing, this should happen almost immediately; we don't need to wait for a KonqRun or anything). I do not like the timer solution either. However, short of calling setFocus() like the previous solution, I see no other way to address the problem. That is because the issue is not knowing whether we are going to open a url in the tab or not, but when. It is a timing issue as to when we should check and change the focus to the locationbar. Right now we do that in the only place that can accommodate all use cases ; both the creation of a new tab through code and the manual creation/activation of the tab by the user. Unfortunately, that is exactly the wrong place to make such a decision for the tab creation through code use case because a tab can be created with blank or empty URL. The only other solution I see is to change the focus back to the view when the KPart emits the started(KIO::Job*) signal. That won't affect the manual tab activation use case and won't require timer. It does however mean adding a setFocus() call. Would that be a preferable solution ? - Dawit --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105984/#review17743 --- On Aug. 20, 2012, 2:22 a.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105984/ --- (Updated Aug. 20, 2012, 2:22 a.m.) Review request for KDE Base Apps and David Faure. Description --- The attached patch address the bug reported in #304933. Right now if Konqueror is configured to open new tabs in the foreground, i.e. the Open tabs in the background option is unchecked, then the keyboard focus is put on the location bar instead of the view. This addresses bugs 304865 and 304933. http://bugs.kde.org/show_bug.cgi?id=304865 http://bugs.kde.org/show_bug.cgi?id=304933 Diffs - konqueror/src/konqframe.h 60aa4d0 konqueror/src/konqframe.cpp 10ed7cd konqueror/src/konqviewmanager.cpp 5352eeb Diff: http://git.reviewboard.kde.org/r/105984/diff/ Testing --- Thanks, Dawit Alemayehu
Re: Review Request: Focus goes to location bar when opening link in new tab in foreground
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105984/ --- (Updated Aug. 20, 2012, 8:02 p.m.) Review request for KDE Base Apps and David Faure. Changes --- Here is the only other option I personally see to handle all the focus related bugs. This solution does not use a timer, but has its own minor flaws. Namely when a new tab is created when a clicks on a link with the MMB for example, the location bar will gain the focus until the part emits its started signal. This might be for a brief moment or for extended period depending on when Konqueror receives the started signal. It does not fix the problem if the part does not send such signal. To be perfectly honest the timer based solution actually works much better visually. There is a tiny delay when activating a blank tab (not creating it), but nothing like what you can observe in this case. Note that the isLoading() check in KonqFrame::activateChild is still needed for the session restore case. Otherwise the focus will be on the location bar in that case as well and as stated previously the calls to emitActivePartChanged() in KonqViewManager::showTab are redundant calls. Description --- The attached patch address the bug reported in #304933. Right now if Konqueror is configured to open new tabs in the foreground, i.e. the Open tabs in the background option is unchecked, then the keyboard focus is put on the location bar instead of the view. This addresses bugs 304865 and 304933. http://bugs.kde.org/show_bug.cgi?id=304865 http://bugs.kde.org/show_bug.cgi?id=304933 Diffs (updated) - konqueror/src/konqframe.cpp 10ed7cd konqueror/src/konqview.cpp db9ffd4 konqueror/src/konqviewmanager.cpp 5352eeb Diff: http://git.reviewboard.kde.org/r/105984/diff/ Testing --- Thanks, Dawit Alemayehu
Re: Review Request: Fix SSL SNI support in KTcpSocket
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106087/#review17783 --- This review has been submitted with commit df1f193ba792b3bd258254ab3f75d4e76b0d0524 by Dawit Alemayehu to branch KDE/4.9. - Commit Hook On Aug. 19, 2012, 6:09 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106087/ --- (Updated Aug. 19, 2012, 6:09 p.m.) Review request for kdelibs. Description --- The attached patch adds two missing and valid flag conversions from KTcpSocket::SslVersion to QSsl::SslProtocol in KTcpSocket's qSslProtocolFromK. This causes TLSv1 SNI support to fail in TcpSlaveBase because it was recently changed by me to use KTcpSocket::SecureProtocols by default. This addresses bug 304212. http://bugs.kde.org/show_bug.cgi?id=304212 Diffs - kdecore/network/ktcpsocket.cpp 8da620a Diff: http://git.reviewboard.kde.org/r/106087/diff/ Testing --- Test site from the bug report: https://sni.velox.ch/ Thanks, Dawit Alemayehu
