Opened 11 months ago

Closed 9 months ago

#25126 closed enhancement (fixed)

about:tor should work on small screens (mobile)

Reported by: igt0 Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-torbutton, TorBrowserTeam201804R
Cc: brade, mcs Actual Points:
Parent ID: #24855 Points:
Reviewer: Sponsor:

Description

Right now the about:tor page doesn't render properly on mobile.

Child Tickets

Attachments (1)

abouttor-mobile320480.png (24.6 KB) - added by igt0 10 months ago.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 11 months ago by igt0

Parent ID: #24855

comment:2 Changed 11 months ago by mcs

Cc: brade mcs added

That does not surprise me. If I remember correctly, the original about:tor HTML and CSS was contributed by a volunteer quite a few years ago. I am not sure if there is a ticket for it yet, but one of the UX team projects for 2018 is to redesign about:tor and work on an "onboarding" flow for our new users.

comment:3 Changed 11 months ago by gk

Component: Applications/TorbuttonApplications/Tor Browser
Keywords: tbb-torbutton added
Owner: set to tbb-team

comment:4 Changed 10 months ago by igt0

Status: newneeds_review

As discussed in the weekly meeting(02/05). It would be interesting to make the current about:tor layout responsive while we are waiting for the new design.

The code can be found here:

bug 25126: Make about:tor layout responsive
https://github.com/igortoliveira/tor-browser/commit/637076be7d34b9aed43715e862b2e14b0a3902a0

and it depends of the #25013 patch set.

Changed 10 months ago by igt0

Attachment: abouttor-mobile320480.png added

comment:5 Changed 10 months ago by mcs

Keywords: TorBrowserTeam201802R added

comment:6 Changed 10 months ago by gk

Keywords: TorBrowserTeam201802 added; TorBrowserTeam201802R removed
Status: needs_reviewneeds_revision

Marking this as needs_revision as the #25013 needs to get revised as well.

comment:7 Changed 10 months ago by igt0

Status: needs_revisionneeds_review

I updated the code:

bug 25126: Make about:tor layout responsive
https://github.com/igortoliveira/torbutton/commit/17b1d031bb64d4c4ad38ab35a1730cda18ac0664

comment:8 Changed 10 months ago by gk

Keywords: TorBrowserTeam201802R added; TorBrowserTeam201802 removed

comment:9 Changed 10 months ago by gk

Keywords: TorBrowserTeam201803R added; TorBrowserTeam201802R removed

Moving our reviews to March 2018

comment:10 Changed 9 months ago by gk

Keywords: TorBrowserTeam201804R added; TorBrowserTeam201803R removed

mcs, brade: can you take a look at that one?

comment:11 Changed 9 months ago by mcs

Keywords: TorBrowserTeam201804 added; TorBrowserTeam201804R removed
Status: needs_reviewneeds_revision

Kathy and I reviewed the changes and tested them using a desktop browser. We like the use of flexbox layout (very nice), but we found a few things that we should be fixed:

  • We could merge this now if it didn't depend on some earlier changes. It is also confusing that the patch "undoes" some earlier changes, so please make a new patch that does not depend on a35a746ca206706f4c5742d70bc90b649c1bfdf0 and 40682676e33570d464534adc07bf94acb36a76fe.
  • The class name container is very generic. Please replace it with a more specific name such as torstatus-container.
  • Removing .top from aboutTor.css causes the "WARNING: this browser is out of date." text to no longer be centered.
  • The fancy arrow that points to the toolbar item to encourage users to update their browser loses its horizontal tail when the browser window is at its default width. Somehow the div that has no id (that immediately follows the #torstatus div) is now wider than it used to be. This may be a consequence of the flexbox layout or just the adding of the .container div.

comment:12 in reply to:  11 Changed 9 months ago by igt0

Status: needs_revisionneeds_review

Thanks Kathy and mcs. Indeed I rebased with the wrong branch. The patch bellow fixes all the issues described by you.

bug 25126: Make about:tor layout responsive

https://github.com/igortoliveira/torbutton/commit/d12165d161817573e59ca0b6effa5d5e8d5cafe0

Replying to mcs:

Kathy and I reviewed the changes and tested them using a desktop browser. We like the use of flexbox layout (very nice), but we found a few things that we should be fixed:

  • We could merge this now if it didn't depend on some earlier changes. It is also confusing that the patch "undoes" some earlier changes, so please make a new patch that does not depend on a35a746ca206706f4c5742d70bc90b649c1bfdf0 and 40682676e33570d464534adc07bf94acb36a76fe.
  • The class name container is very generic. Please replace it with a more specific name such as torstatus-container.
  • Removing .top from aboutTor.css causes the "WARNING: this browser is out of date." text to no longer be centered.
  • The fancy arrow that points to the toolbar item to encourage users to update their browser loses its horizontal tail when the browser window is at its default width. Somehow the div that has no id (that immediately follows the #torstatus div) is now wider than it used to be. This may be a consequence of the flexbox layout or just the adding of the .container div.

comment:13 Changed 9 months ago by gk

Keywords: TorBrowserTeam201804R added; TorBrowserTeam201804 removed

comment:14 Changed 9 months ago by mcs

Keywords: TorBrowserTeam201804 added; TorBrowserTeam201804R removed
Status: needs_reviewneeds_revision

This looks good, except we found one additional problem: using a fixed size for the bubble elements does not work for all languages. We tested by setting general.useragent.locale to de and found that the text extended below the bubble background. Kathy and I removed the height rule and that seemed to fix the problem, but maybe there is a better solution.

Also, please update the year within the copyright notice of both files.

comment:15 in reply to:  14 ; Changed 9 months ago by igt0

Keywords: TorBrowserTeam201804R added; TorBrowserTeam201804 removed
Status: needs_revisionneeds_review

Replying to mcs:

This looks good, except we found one additional problem: using a fixed size for the bubble elements does not work for all languages. We tested by setting general.useragent.locale to de and found that the text extended below the bubble background. Kathy and I removed the height rule and that seemed to fix the problem, but maybe there is a better solution.

In fact I think it is a good solution, thus the height of the higher bubble item will define the height of the parent element. I gave a try and tested in different languages and screen size, and looks good.

Also, please update the year within the copyright notice of both files.

nice catch, thanks.

Updated code:

bug 25126: Make about:tor layout responsive
https://github.com/igortoliveira/torbutton/commit/bcc6dc11e5c9bed40ffe976ab93ec6da17ee33ea

comment:16 Changed 9 months ago by mcs

r=brade, r=mcs
This looks good to us. It probably makes sense to test it in the desktop alpha before merging into stable though (Kathy and I only tested the new code on macOS so far).

comment:17 in reply to:  15 Changed 9 months ago by cypherpunks

Replying to igt0:

https://github.com/igortoliveira/torbutton/commit/bcc6dc11e5c9bed40ffe976ab93ec6da17ee33ea

tbb-team, open it with Safest and look at your CPU!

comment:18 in reply to:  15 ; Changed 9 months ago by gk

Replying to igt0:

Replying to mcs:

This looks good, except we found one additional problem: using a fixed size for the bubble elements does not work for all languages. We tested by setting general.useragent.locale to de and found that the text extended below the bubble background. Kathy and I removed the height rule and that seemed to fix the problem, but maybe there is a better solution.

In fact I think it is a good solution, thus the height of the higher bubble item will define the height of the parent element. I gave a try and tested in different languages and screen size, and looks good.

Also, please update the year within the copyright notice of both files.

nice catch, thanks.

Updated code:

bug 25126: Make about:tor layout responsive
https://github.com/igortoliveira/torbutton/commit/bcc6dc11e5c9bed40ffe976ab93ec6da17ee33ea

I get merge conflicts for some reason. Could you rebase your patch on latest master. Please use a new branch for that, thanks.

comment:19 in reply to:  18 Changed 9 months ago by igt0

Here you go (25126-rebased branch):

https://github.com/igortoliveira/torbutton/commit/703b228d75dfb0d7a44de4ad5a056406e44691e5

Replying to gk:

Replying to igt0:

Replying to mcs:

This looks good, except we found one additional problem: using a fixed size for the bubble elements does not work for all languages. We tested by setting general.useragent.locale to de and found that the text extended below the bubble background. Kathy and I removed the height rule and that seemed to fix the problem, but maybe there is a better solution.

In fact I think it is a good solution, thus the height of the higher bubble item will define the height of the parent element. I gave a try and tested in different languages and screen size, and looks good.

Also, please update the year within the copyright notice of both files.

nice catch, thanks.

Updated code:

bug 25126: Make about:tor layout responsive
https://github.com/igortoliveira/torbutton/commit/bcc6dc11e5c9bed40ffe976ab93ec6da17ee33ea

I get merge conflicts for some reason. Could you rebase your patch on latest master. Please use a new branch for that, thanks.

comment:20 Changed 9 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Thanks. Applied to master (commit fb8c46b3fb3e89b0fd182f87ba3139f5b31ad186).

Note: See TracTickets for help on using tickets.