Skip to content
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

show start orbot dialog #2834

Closed

Conversation

harshitbansal05
Copy link

This pr tries to solve #1980. It checks the tor status availability by already present account status callbacks in ConversationActivity . It shows a dialog if orbot is not started, and places a gap of a hour between two dialogs to prevent spam.

@licaon-kter
Copy link

licaon-kter commented Feb 24, 2018

Some ideas:

  • you are not covering the case where Orbot is not installed
  • you are not covering the ADD ACCOUNT case? This feels most important.
  • text/buttons colour (blue? accent?) does not match app themes (green/white or green/dark grey)
  • 60 minutes is too long, what if I dismiss it by mistake? On each reconnect try I might need to see this.

I would like the dialogue simplified, here's the one from Chat Secure (it has a dark theme too) which I find good enough:
pic

Else, here are my issues with the current one:

Start Orbot/Uncheck Tor

What's the need for the text below if the title says it all? :)

Looks like Orbot is not started on your phone.

What phone? This a tablet. :) Maybe make it device ?

Would you like to start it or uncheck Tor?

Umm, ok

NO, THANKS

NO what? Better name it Ignore (eg. Dismiss the dialogue, I'll see my self if there's a problem with Orbot; or maybe I don't understand what's going on)

START ORBOT

Ok, clear

UNCHECK TOR

Not clear what uncheck does, will it connect without Tor? If I chose Tor for my safety this is bad (!?)

Regarding the not installed case, it needs another dialogue:

  • title: Install Orbot?
  • content: You must have Orbot installed and activated to proxy traffic through it. Would you like to install it? OK/Cancel
  • OK starts an intent to market://details?id=org.torproject.android which will prompt F-Droid or Google Play or a browser I guess.

@licaon-kter
Copy link

That's the cosmetic things, the main thing is that your START does not actually do anything.

Did this work on your device? Here it does not, while ChatSecure's intent trigger for: org.torproject.android.START_TOR works fine, see the linked file in the original issue. ;)

@harshitbansal05
Copy link
Author

@licaon-kter clicking on START should open Orbot, and it works fine on my mobile. Is'nt it working fine on yours?

@licaon-kter
Copy link

licaon-kter commented Feb 25, 2018

Obviously not.

Try the other intent to not only open the app, but start the Tor router too.

@harshitbansal05
Copy link
Author

@licaon-kter just two more things:

  1. Sorry, but I could'nt understand what you meant by ADD ACCOUNT case. Does it refer to the case when no account is added?
  2. Should I remove the uncheck Tor option(it connects without Tor and unchecks it from Settings)?
    Thanks

@licaon-kter
Copy link

  1. When you install the app for the first time, use my provider, settings, enable use Tor, then add account...

  2. I guess, if it wants it disable let the user go to settings and disable it

@harshitbansal05
Copy link
Author

so, @licaon-kter you mean that before moving to the add account activity, i must direct user to the settings where useTor should be enabled by default. The user can then change the setting accordingly and move ahead to the add account activity?
Sorry once again..

@licaon-kter
Copy link

No, the user enables Tor before adding an account, but then your dialogue does not pop up.

I was pointing out how you can test: install the app for the first time, use my provider, settings, enable use Tor, then add account.

@harshitbansal05
Copy link
Author

harshitbansal05 commented Feb 26, 2018

@licaon-kter I am unable to find a solution, when no account is added. Can I use the same method to check tor status as in the file you mentioned in the issue with appropriate modifications?

@licaon-kter
Copy link

licaon-kter commented Feb 26, 2018

You be the judge of that.

@harshitbansal05
Copy link
Author

@licaon-kter could you please review this?

@licaon-kter
Copy link

licaon-kter commented Mar 7, 2018

Somewhat working but not really.

My experience:

  • on clean app install -> no detection of Tor as stopped (only text error message in account screen)
  • start Tor, add account, connect, open a 1:1 chat, got "Start Tor" dialogue... why? It was running already! (albeit pressing start did trigger an Orbot router restart, afaics)
  • while connected, stop Tor, nothing happens (only text error message in account screen)
  • uninstall Orbot, nothing happens (only text error message in account screen)

I bet the timeout has something to do with this (no I won't wait 15mins for this), can you just lower it to 5secs or something?

Was it triggering on your device every time its needed?

@harshitbansal05
Copy link
Author

@licaon-kter regarding the no account case, do I have to show dialog when user clicks connect or do I have to show it periodically as in ConversationActivity. @licaon-kter also, the last two issues would be solved by reducing the delay time. I would do so.

@harshitbansal05
Copy link
Author

@licaon-kter sorry to disturb you, but could you please guide?

@licaon-kter
Copy link

See the conditions when "tor network error" red text is shown in accounts, and wire those to the dialogue.

@iNPUTmice
Copy link
Owner

Yeah I don't think this needs to be a full dialog that pops up and negs the user. Probably just replace the [Connect] button in the accounts screen with a start orbot button or something like that (we do this for 'open website' when registration failed for example.

@harshitbansal05
Copy link
Author

@iNPUTmice I have done the changes, could you please review?

@harshitbansal05
Copy link
Author

@licaon-kter could you review it please?

@licaon-kter
Copy link

So, clean app data.
New account, settings, Tor
User&Pass
START ORBOT
Triggers Orbot start
...and nothing else...each time I press it triggers the same action
...it does not detect that the network is already on so it never connects (nor does it try)

tested with: Orbot 16.0.0-RC-2-multi-SDK23

@harshitbansal05
Copy link
Author

@licaon-kter do you mean that the START ORBOT button always persists? even if orbot is on?

@licaon-kter
Copy link

Yes.

Did it work for you? What Orbot version?

@harshitbansal05
Copy link
Author

Yes, it did. I was using the same orbot version @licaon-kter. Does it correct after some time, or it remains in the same START ORBOT state? Also, did the conversation activity work fine?
btw, Thanks for your support

@licaon-kter
Copy link

Stays there.

Does not do anything, stuck at user/pass/server view, with CANCEL/START ORBOT, while Orbot is running.

@harshitbansal05
Copy link
Author

ok, I would look into it.. also, is the Conversation Activity working fine @licaon-kter

@licaon-kter
Copy link

What do you mean by "conversation activity works fine" ?

@harshitbansal05
Copy link
Author

The start orbot dialog that had to be shown in ConversationsActivity @licaon-kter

@harshitbansal05
Copy link
Author

@licaon-kter as @iNPUTmice mentioned,

"Yeah, I don't think this needs to be a full dialog that pops up and negs the user. Probably just replace the [Connect] button in the accounts screen with a start orbot button or something like that (we do this for 'open website' when registration failed for example."

The dialog won't be shown in AddAccount activity. I was talking about the ConversationsActivity where all the contacts are shown, the main screen.
Also, I think the previous commit must solve the problem. Could you kindly review?
Thanks

@licaon-kter
Copy link

I'll rebuild later. Thanks

@harshitbansal05
Copy link
Author

@licaon-kter sorry to disturb, but have you checked it?

@harshitbansal05
Copy link
Author

@iNPUTmice could you please review?

@licaon-kter
Copy link

Nothing new, still stuck there.

Shouldn't it detect Tor as connected and replace the START ORBOT with CONNECT or something? If so, looks like you don't detect that is connected (already).

@harshitbansal05
Copy link
Author

@licaon-kter may I know the android version you are using. I can't figure it out, it works fine on my phone..

@licaon-kter
Copy link

Oreo

@harshitbansal05
Copy link
Author

@licaon-kter Starting from noughat, android restricts finding process id of other apps, as a result it always stated the app as not started. I have made the changes to determine tor status without using shell commands. Hopefully, it would work and sorry for all the trouble you beared till now..
Thanks

@harshitbansal05
Copy link
Author

@licaon-kter could you please review it?

@harshitbansal05
Copy link
Author

@licaon-kter sorry to disturb, could you please review?

@licaon-kter
Copy link

On add account, working ok, sees Orbot not started, starts it.

On account already connected, stopping Orbot, stuck with "tor network is not available".

@harshitbansal05
Copy link
Author

On account already connected, stopping Orbot, stuck with "tor network is not available".

@licaon-kter does this refer to the case after user has created an account?

@licaon-kter
Copy link

Yes, say I'm connected for a while and my dumb ROM optimizes my battery by killing Orbot while my phone is in my pocket.

@harshitbansal05
Copy link
Author

@licaon-kter I thought, it had to be shown only for the Add Account case. For managing/editing accounts, no change would occur. It would follow the same old flow..

@licaon-kter
Copy link

Oh, ok, I for one would like for Conversations to retry to enable/start Orbot on connection retry. That's useful.

@harshitbansal05
Copy link
Author

@licaon-kter i like the idea but imo, it would be better to open an issue for that and do it a separate pull request. what do you think?
Also, does this pr works fine for the intended task (displaying dialog in ConversationsActivity and Add Account case)?

@licaon-kter
Copy link

I did not see any "dialogue" in Add account

Not sure what is ConversationActivity, normal chat view?

@harshitbansal05
Copy link
Author

@licaon-kter in Add Account activity, only the button would change if orbot is'nt working, no dialog would be shown. And yes, ConversationActivity is activity where all the contacts are seen.

@licaon-kter
Copy link

No, no dialogue at all.

Also, you contradict yourself, either "For managing/editing accounts, no change would occur. It would follow the same old flow.." or "And yes, ConversationActivity is activity where all the contacts are seen."...no dialogue is ever displayed if Orbot is stopped (except the button text on Add account that is)

@harshitbansal05
Copy link
Author

@licaon-kter not sure why it is'nt working in main chat view.. works fine in lollipop and marshmellow.

@licaon-kter
Copy link

Ok, looks I was not in the correct activity, indeed it works when you already have a chat open, I was in the contacts or accounts activity.

Now, the message should be rephrased, and remove or uncheck Tor since the options are OK (start Orbot) and CANCEL (do nothing)?

Also, after pressing OK maybe add a timeout, because it takes a bit until it connects so you'll get the dialogue multiple times when you switch chats during this phase.

@harshitbansal05
Copy link
Author

@licaon-kter I have made the suggested changes...

@licaon-kter
Copy link

Ok, let me rephrase that.

The problem is that before the 15 seconds pass, if you change chat or open a MUC from a bookmark you'll get the dialogue again.

@harshitbansal05
Copy link
Author

@licaon-kter I don't know why this is happening, the dialog should again show only after 15 sec. To be clear, 15 sec duration is from the opening of the dialog and not closing..

@licaon-kter
Copy link

Yes, chat 1, dialogue, press OK, chat list, +, MUCs bookmarks, open MUC1, dialogue shown again.

These steps can be done in 2 seconds...

@harshitbansal05
Copy link
Author

@licaon-kter sorry, but I could not reproduce the issue..
@iNPUTmice could you please review this?

@iNPUTmice iNPUTmice closed this in 875810e Dec 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants