BUG: 354246
Details
- Reviewers
dfaure cfeck - Group Reviewers
Frameworks - Commits
- R126:484a7dc89a75: [KRun] Don’t follow redirection to speed up and avoid incorrect behavior
Launch kde-open5 <URL to a website you have an account and that redirects when not connected>, e.g. https://twitter.com/i/notifications.
If it works, we see the correct URL, and not https://twitter.com/login?redirect_after_login=%2Fi%2Fnotifications.
Diff Detail
- Repository
- R126 KDE CLI Utilities
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Can you change Fix https://bugs.kde.org/show_bug.cgi?id=354246 to BUG: 354246? Thanks!
See https://community.kde.org/Infrastructure/Phabricator#Add_special_keywords
It would also be nice if you could update the Test Plan section with an indication of how you tested this change.
src/widgets/krun.cpp | ||
---|---|---|
1319 | You cannot remove it, since it can be a local file, remote file with scheme that differs http and should be redirect then. So you can make it like that if (!job->url().scheme().startsWith("http")) { setUrl(job->url()); } Now it will not redirect only http and https. |
src/widgets/krun.cpp | ||
---|---|---|
1319 | Also you can add Qt::CaseInsensitive as second parameter to startsWith for scheme with capital letters. |
I'm really not convinced about this. What about all the cases where you do want to see the redirection happen?
- Redirection from http to https
- Redirection from some outdated URL to the new correct URL
- Redirection from google.com to www.google.com
I think this needs more research. Why do other browsers show the new URL in all of the above cases and not for the twitter testcase? Different HTTP error code maybe?
(I vaguely remember something about permanent redirections vs normal redirections?)
src/widgets/krun.cpp | ||
---|---|---|
1319 | Not necessary, scheme() always returns the scheme lower-cased. |
OK after reading the bug report, I think this is more about "KRun as used by kde-open5 vs KRun as used by other apps such as konqueror".
We want e.g. konqueror to follow the redirection. On the other hand we don't want kde-open5 to follow the redirection. Then what we might need is a KRun::setFollowRedirections(bool). On by default, set to off by kde-open5.
Is anybody interested in helping me figure out how I can implement that properly? I would be glad to start working again on it, but I don’t know enough about KIO right now.
Uses new function added in D17726
The commit message I made contains, after title:
Often, trying to access a restricted webpage redirect to a login page.
If you’re connected to the website, you don’t want to be redirected.Before, kde-open5 followed the redirection and then opened URL.
Now, it opens the browser as soon as possible and open the correct page.
Please add a KIO version ifdef to avoid breaking compilation with older versions of KIO.
@dfaure I don’t see how to do that, since there does not seem to have such ifdef in the actual codebase. However I can make set(KF5_MIN_VERSION "5.55.0") instead of 5.54.0 in the CMakeLists.txt.
Can you provide your email address so we can land this patch for you with proper authorship information? Thanks!