[tor-bugs] #28051 [Applications/Tor Browser]: Build Orbot into TBA

Tor Bug Tracker & Wiki blackhole at torproject.org
Tue Nov 27 04:19:38 UTC 2018


#28051: Build Orbot into TBA
-------------------------------------------------+-------------------------
 Reporter:  sysrqb                               |          Owner:  sysrqb
     Type:  enhancement                          |         Status:
                                                 |  needs_revision
 Priority:  Very High                            |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-mobile, tba-a2,                  |  Actual Points:
  TorBrowserTeam201811                           |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
                                                 |  Sponsor8
-------------------------------------------------+-------------------------

Comment (by sysrqb):

 Replying to [comment:30 gk]:
 > `28051_5` looks good to me now. Here comes the review of the Orbot
 patches:

 Thanks! I'll clean these up and prepare a new branch.

 >
 > 42ce2f3be79078a780f80aa6b3d5bdcf7d685737 - not okay "that that" (one
 "that" is enough); I guess we could ignore the minimalperm
 `AndroidManifest.xml` as there is no such productFlavor anymore anyway? (I
 was confused as you seemed to not adjust the
 targetSdkVersion/mcSdkVersion)?

 Yes, I agree. I didn't dig into the history of the minimalperm manifest,
 but now I see why it is there. I agree we can ignore it because it isn't
 being used anymore.

 >
 > 9ee57d5e158ec0f8c5dc71decfa4eb5d7656e70d - okay
 >
 > 108a70748ce6a2406bc780fca752c24801b777a3 - okay
 >
 > 60e599b9fdbca45ff2dababf5fcaa791aabcf654 - do we need minimalperm
 changes?

 Nope.

 >
 > 653a7f6f119b26dd7920ea802435d3b3163ec19d - okay (what's the reason we
 suddenly need to cast them?)
 >

 I get an error like:

 {{{

 > Task :app:compileFullpermReleaseJavaWithJavac FAILED
 /home/android/orbot/app/src/main/java/org/torproject/android/ui/AppManagerActivity.java:62:
 error: incompatible types: View cannot be converted to GridView
         listApps = findViewById(R.id.applistview);

 }}}

 I don't know the exact reason, but this is either directly or indirectly
 caused by changing the support library from version 27.1.1 to 23.4.0. I
 thought this was related to the API/SDK version we were targeting or
 compiling, but changing those doesn't affect whether or not we receive a
 compile-time error. I only see this when changing the version of
 `implementation 'com.android.support:design:23.4.0'` in app/build.gradle.

 > b82c86f18775d55fe60ace7bdb1a7b2f5d1f70c0 - if you grep for
 "LocaleHelper" there are a bunch of other cases you did not touch. Are we
 good with leaving them as-is?

 It would be safer deleting the other cases, as well.

 >
 > c487b13bfc21a2e5ececdf0f5ad151afef011cf5 - "// Rename preferences do it"
 -> "//Rename preferences so it"; you could think about patching the
 WALKTROUGH file as well to change "preferences.xml" there, too (but I
 don't feel strongly about that)
 >

 Keeping the documentation consistent is a good idea.

 > c3e4361f4b8b6366aefd14dc3dfcb8e1f41badc9 - could you add a comment why
 you commented the `setChannelId()` call? (in case we really need to
 comment it out); actually after looking at patch 12 it seems this part of
 the patch should get deleted. It's already commented out in patch 12 (that
 is
 > b529abde51aac1a0a7fa6ca2500313a2e13286fc, where it fits better).

 Agreed.

 >
 > ab2e00148dd7b522d34c0a38bd2f2efaefe62683 - "swipped away" -> "swiped
 away"
 >

 Ack.

 > b29a86b6b8d0ec9020ff96592bdd84ae6a899d30 - okay
 >
 > 0effce6c167ad8ccc03ca476b6e1db6ddecbea7d - okay
 >
 > b529abde51aac1a0a7fa6ca2500313a2e13286fc - "This was add" -> "This was
 added"; how did you chose the methods to implement in your wrapper class?
 There seem to be other methods that got not implemented and not used and
 others implemented and not used in Orbot's code.

 Ack. I chose those methods because those should be all the methods used by
 used by Orbot and Fennec when they create notifications. My hope is this
 will provide us with notifications on all supported platforms. If I missed
 any methods expected by the app, then we should implement them.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/28051#comment:31>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online


More information about the tor-bugs mailing list