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

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#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
-------------------------------------------------+-------------------------
Changes (by gk):

 * keywords:  tbb-mobile, tba-a2, TorBrowserTeam201811R => tbb-mobile,
     tba-a2, TorBrowserTeam201811
 * status:  needs_review => needs_revision


Comment:

 `28051_5` looks good to me now. Here comes the review of the Orbot
 patches:

 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)?

 9ee57d5e158ec0f8c5dc71decfa4eb5d7656e70d - okay

 108a70748ce6a2406bc780fca752c24801b777a3 - okay

 60e599b9fdbca45ff2dababf5fcaa791aabcf654 - do we need minimalperm changes?

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

 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?

 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)

 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).

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

 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.

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

_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs