TBA: Implement about:tor
Onion circles implemeted in html + css:
https://trac.torproject.org/projects/tor/attachment/ticket/25696/onion_circles_pattern.html
Onion circles implemented in svg:
https://trac.torproject.org/projects/tor/attachment/ticket/25696/onion_circles_pattern.py
.png and .svg version were ready to be exported at https://marvelapp.com/5981a4b/screen/44384308/handoff
more info: https://trac.torproject.org/projects/tor/ticket/25696#comment:15
Activity
Trac:
Parent: N/A to #26531 (moved)
Cc: sysrqb, gk to sysrqb, gk, igt0Bug 27111 - Update about:tor desktop version to work on mobile https://github.com/igortoliveira/torbutton/commit/6ccd196ed2cf54d14fcb661cfdff0c59626191cd
Trac:
Status: new to needs_reviewMoving to second-alpha TBA keyword.
Trac:
Keywords: N/A deleted, TBA-a2 added
Parent: #26531 (moved) to N/APartially unrelated, but ideally, when the new tb-manual is available for TBA, then we'll probably need to adjust the URL construction within
src/chrome/content/aboutTor/aboutTor-content.js:onLocaleChange(). (I guess this is similar to option 1 in #20739 (moved)). We'll need a new mockup for this, too.
In
src/chrome/content/torbutton.js:torbutton_init():+ if (torbutton_is_mobile()) { + torbutton_abouttor_message_handler.updateAllOpenPages(); + } +Can you add a comment about why this is needed only on mobile?
We should set the homepage as
about:tor, too. Fennec doesn't respectbrowser.startup.homepage, so we must hardcode it :/The easiest option may be modifying
mobile/android/base/java/org/mozilla/gecko/AboutPages.java:HOME. Unfortunately, this won't work well until torbutton is integrated and there's no longer the need of restarting the app before torbutton fully installs. Another option is adding in a check forbrowser.startup.homepage, and that overridesAboutPages:HOME.
This patch doesn't provide the same view as the mockup with the virtual keyboard opened (right-side of https://trac.torproject.org/projects/tor/attachment/ticket/25696/24309%20-%20TBA%20-%20Welcome%20Page.png). This'll require a tor-browser patch, as well, because Fennec automatically opens the ActivityStream HomePager every time the URL bar is focused.
Overall, I don't see any problems with this torbutton patch. This should be reviewed by someone else (in particular someone with more knowledge of this code) - making sure this doesn't break anything on desktop. I'm setting to needs_revision because I think explaining why about:tor requires an explicit signal on mobile during init() is helpful for reviewing this.
Trac:
Status: needs_review to needs_revisionI updated the patch:
https://github.com/igortoliveira/torbutton/commit/f5953aec52e8f9662c45cfff60cfe0b775ba43cb
and I replied the comments bellow:
Replying to sysrqb:
Partially unrelated, but ideally, when the new tb-manual is available for TBA, then we'll probably need to adjust the URL construction within
src/chrome/content/aboutTor/aboutTor-content.js:onLocaleChange(). (I guess this is similar to option 1 in #20739 (moved)). We'll need a new mockup for this, too.Oh nice catch. I will talk with Antonela about it.
In
src/chrome/content/torbutton.js:torbutton_init():+ if (torbutton_is_mobile()) { + torbutton_abouttor_message_handler.updateAllOpenPages(); + } + }}} Can you add a comment about why this is needed only on mobile?Left over. I removed it.
We should set the homepage as
about:tor, too. Fennec doesn't respectbrowser.startup.homepage, so we must hardcode it :/The easiest option may be modifying
mobile/android/base/java/org/mozilla/gecko/AboutPages.java:HOME. Unfortunately, this won't work well until torbutton is integrated and there's no longer the need of restarting the app before torbutton fully installs. Another option is adding in a check forbrowser.startup.homepage, and that overridesAboutPages:HOME.You are right. It is why I have a second patch for this bug where I add the following file to tor browser:
mobile/android/torbrowser/assets/distribution/preferences.json
{{{
{ "ApplicationPreferences": { "newtab.load_homepage": true }, "AndroidPreferences": { "homepage": "about:tor", "startpane_enabled_after_57": true, "startpane_enabled": true } }
> ---- > > This patch doesn't provide the same view as the mockup with the virtual keyboard opened (right-side of https://trac.torproject.org/projects/tor/attachment/ticket/25696/24309%20-%20TBA%20-%20Welcome%20Page.png). This'll require a tor-browser patch, as well, because Fennec automatically opens the ActivityStream HomePager every time the URL bar is focused. > Yes, In a follow up patch, I will remove the following method call from the BrowserApp.javashowHomePagerWithAnimator(panelId, null, animator);
> ---- > > Overall, I don't see any problems with this torbutton patch. This should be reviewed by someone else (in particular someone with more knowledge of this code) - making sure this doesn't break anything on desktop. I'm setting to needs_revision because I think explaining why about:tor requires an explicit signal on mobile during init() is helpful for reviewing this. **Trac**: **Status**: needs_revision **to** needs_reviewTor Browser patch to make about:tor to show up:
I'll do a proper code review tomorrow with hopefully more brain available. However, here is some feedback. Strings are clipped on a Samsung Galaxy S5 mini and a bit misaligned if I have the browser open vertically (see: attached screenshot). Horizontally it looks pretty good.
Git is complaining about a new blank line at the end of `aboutTor.css. We should remove it.
More to come tomorrow...
Trac:
Status: needs_review to needs_revisionIt is a left over from a rebase conflict. I updated the patch removing the inner torstatus-version and changing the style:
https://github.com/igortoliveira/torbutton/commit/08eff9958ff9035290501180bdeef2787f3e0971
Replying to gk:
What is
body[mobile] .torstatus-version,doing given that there is noclass="torstatus-version"as far as I can see? And why do we have a second<div id="torstatus-version"/>now?Trac:
Status: needs_revision to needs_review
