[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #26884 [Applications/Tor Browser]: Update preferences.xul to make it work on mobile



#26884: Update preferences.xul to make it work on mobile
-------------------------------------------------+-------------------------
 Reporter:  igt0                                 |          Owner:  tbb-
                                                 |  team
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Very High                            |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-torbutton,                       |  Actual Points:
  TorBrowserTeam201808R                          |
Parent ID:  #26531                               |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by sysrqb):

 * cc: arthuredelstein (added)
 * status:  needs_review => needs_revision


Comment:

 Replying to [comment:3 igt0]:
 > XUL doesn't work well on mobile, so I implemented the mobile preferences
 in XHTML.
 >
 > Initially, I tried to make the code reusable across the mobile and
 desktop, however I was losing lot of time trying to make the Desktop
 version work. So this patchset has just the **mobile** implementation.
 >

 The original XUL implementation still works on Desktop, correct? Only
 mobile uses XHTML?

 > You can see the patches here:
 > https://github.com/igortoliveira/torbutton/commits/26884
 >
 > **Bug 26884 - Part 1: Move show_torbrowser_manual and
 get_general_useragent_locale to utils**
 >
 https://github.com/igortoliveira/torbutton/commit/07382c5ee23470bbc08a785c2b349fdb06010696
 >

 Seems okay - but **Arthur**, maybe you want to skim through one?

 ----

 > **Bug 26884 - Part 2: Create mobile security slider**
 >
 https://github.com/igortoliveira/torbutton/commit/6dfbf8bc311208a14f146f80fb285dcd5efc3f45
 >

 Missing (optional) semi-colon - but all the other lines end with a semi-
 colon:
 {{{
 +// Set the desired slider value and update UI.
 +function torbutton_set_slider(sliderValue) {
 +  state.slider = sliderValue
 }}}

 (preferences.js is missing a semicolon on this line, too)

 ----

 Unnecessary semi-colon after the closing curly bracket of a few functions.
 It seems like those may be copied from `preferences.js`.

 ----

 `descNames` and `linkNames` are reversed in preferences-mobile.js
 (compared to preferences.js). Can you add a comment about this?

 ----

 `SECURITY_PREFERENFES_URI` should be `_PREFERENCES_`?

 ----

 > **Bug 26884 - Part 3: Remove optionsURL from install.rdf**
 >
 https://github.com/igortoliveira/torbutton/commit/973b138dd24cd193b990c43c8c9eaa221a8d55b4

 Seems okay.

 Do we need to add fennec as a new target application in install.rdf?
 {{{
 +        <!-- fennec -->
 +        <em:targetApplication>
 +            <Description>
 +                <em:id>{aa3c5121-dab2-40e2-81ca-7ea25febc110}</em:id>
 +                <em:minVersion>60.0</em:minVersion>
 +                <em:maxVersion>10000.0</em:maxVersion>
 +            </Description>
 +        </em:targetApplication>
 }}}

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/26884#comment:9>
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