-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make FORCE_WEBSOCKETS option configurable at runtime (not build-time) #40
base: feature/websocket-reborn
Are you sure you want to change the base?
Make FORCE_WEBSOCKETS option configurable at runtime (not build-time) #40
Conversation
Why should users ever want to switch? Imho that is a developer only option and should therefore not be exposed to users. |
@schachmat I disagree. I want to have one universal LibreSignal build useful on both GCM/non-GCM devices and I want to be able to change this setting at runtime (without reinstallation) to compare battery stats, try it on microG instead of Google Play Services, etc. Option is hidden in Advanced settings menu, so it should not confuse users. And I am really not a fan of removing options and dumbing down apps (iOS / GNOME style)... |
Since it is one of the basic ideas of TextSecure to focus on simplicity, I think giving the user an option to change the message delivery channel would not help being accepted upstream. Reading WS issue tracker, moxie is strongly against additional user options. IMHO, developer related (low-level) options should stay in a developer environment. |
@art1fa Well, like I said above I don't agree with this. And seeing Moxies attitude, I don't believe anymore that WebSocket support will ever get merged upstream. But of course final decision is on @JavaJens ;-) |
👎 I've got to agree with @art1fa... |
Maybe you can take a look at the discussion in #28 I got to be honest, I don't know what to do with this. Maybe, @f41c0r as the steady contributor of upstream can weigh in. I just started this fork because I though it should be done. But I don't use it or (sadly) actively work on it. So I don't know if I am the right one to make any calls. |
@JavaJens People are using your fork because: I agree with @art1fa that moxie will not want to include this commit, but since moxie will not include WebSocket in Signal and doesn't want to put his app on F-droid (and doesn't want any forks under the name Signal), I don't see any reason why this PR is ignored. |
@art1fa @JavaJens All conversations about WebSocket and Signal on F-droid are locked and when I opened a new one, it was almost immediately closed by moxie with a strange comment. |
I still have hope that we can get merged back into upstream in some fashion. I don't have a strong feeling one way or another on this PR. I don't know what Moxie would think of this feature, but I do think it is more convenient for end users interested in this fork than the current status quo. On the other hand, if moving this into a preference means you can't set that preference prior to registration with the TS service, then I'm not a fan of this PR. In older versions of TS, you could not register with the service and only use it for SMS/MMS - presumably then you could flip that preference, and then register with the service. I'd want to check they kept that functionality. Even if they did, is that really intuitive enough of a process as opposed to compiling from source or specifically installing a websockets only apk. |
@f41c0r I think this fork will never be merged upstream. Moxie doesn't want to merge it and doesn't even want to explain why. |
He only explained why he didn't want Signal to be distributed outside of the PlayStore, but I'm not convinced by his explanation. |
@mimi89999 I don't think your judgment is fair here. I am not too certain that this will eventually be merged upstream either, but to be fair: we haven't even tried. Furthermore Websockets and F-Droid are two different things. |
@JavaJens Maybe we could move the discussion about this fork being merged upstream somewhere else? |
@mimi89999 why are you always speaking for moxie? Are you his ambassador extraordinary and plenipotentiary? I don't think so. Why are you referencing #22 with a "we"? I cannot see any contribution or help from you there… If you think a little bit, maybe you will understand the answer from signalapp#4430 I give you a hint: You are annoying people. |
What I wanted to say is that it would be great to allow users to change from GCM to WebSocket and vice-versa (for various reason: privacy, comparing performance) without having to rebuild the app. |
IMHO of course. |
I came here to ask for a feature to automatically switch to websocket if gcm-server is not available for too long (eg. 15 Minutes). But reading this issue I guess this would not be welcome… |
@jensMF I think this is a separate issue and could (potentially) be better suited at upstream as most(?) people here, probably don't even habe GCM. |
I use Signal Jolla Edition, which is built off of this code, and would definitely love to see a clearer situation in these builds. I recently heard of LibreSignal which afaict (without caring to look) is just a rebuild of upstream to circumvent distribution. Now that websockets are a real deal in the server, there's no actual reason (apart from "we want to distribute only on Google and depend heavily on Google") not to merge this. Forking is a good way to develop, test and polish new features before having them merged and IMO having websockets as a toggle makes merging much more plausible, and maintainership of the fork probably easier, and at least the situation clearer. |
As long as there is no official version with websocket support, I second this pull, as I'd more likely would test a version that has this switch than one that does not. I agree this is perhaps not intended to ever get merged upstream, but for the Libre build this is a great thing to do (also in the light to not need to build and maintain 2 branches anymore). |
@xmikos You are the one who currently builds and publishes websocket LibreSignal, right? Have you another branch that you rebase to this one or are you adding own patches? I'd love to get some insight how we could streamline the process and make it a bit more verifyable. Perhaps you also could solve the merge conflicts, so we could get some progress here :)? @JavaJens What are you thinking? |
@Diapolo Yes, I am maintainer of https://fdroid.eutopia.cz repository. I don't have some private fork, I am building it always directly from this JavaJens GitHub repo (I only do automated patching that renames strings from Signal to LibreSignal), you can find my build scripts here: https://github.com/xmikos/fdroiddata I always try to review diffs between previous version and latest version before publishing builds in my repository. |
// TextSecurePreferences.getGcmRegistrationId(this))); | ||
//} | ||
accountManager.getAccountVerificationToken(); | ||
String verificationToken = accountManager.getAccountVerificationToken(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comment or explain what this does? I guess you are allowing RedPhone, when using GCM and still disabling it, if WebSocket is enabled? I like that behaviour, as on phones, which have GCM installed one could activate GCM support to do a RedPhone call and revert after, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you are allowing RedPhone, when using GCM and still disabling it, if WebSocket is enabled?
Yes, this is behavior I want to achieve. But this whole patch is not included in my builds on https://fdroid.eutopia.cz yet (because I am building it directly from this JavaJens repository).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO it would be a good thing to have a coordinated place for LibreSignal, I don't think users want to follow different custom builds and releases. So @JavaJens Could we get this integrated here at first?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So @JavaJens Could we get this integrated here at first?
The consensus here seems to be that this (JavaJens) repository shouldn't diverge much from upstream because there is still hope that Moxie will some day merge it back. Unfortunately I have given up this hope, Moxie is extremely hostile to anything that is not distributed by him on Google Play (and websocket support goes directly against his "walled garden" wishes).
@xmikos Thanks for your explanation :). Thumbs up for your work, you're doing with this. As you may have seen I'm starting to get interested in this fork. |
This is simple change that will make it possible to use WebSocket instead of GCM even on devices which have Google Play Services installed. Now it is needed to maintain two builds (one with FORCE_WEBSOCKETS build-time option enabled and one without it) for this purposes, this would make only one universal build sufficient.
You will find the Force WebSocket option in Advanced Settings.