Opened 3 months ago

Closed 2 months ago

Last modified 8 weeks ago

#31300 closed defect (fixed)

Modify Tor Launcher so it is compatible with ESR68

Reported by: mcs Owned by: brade
Priority: Medium Milestone:
Component: Applications/Tor Launcher Version:
Severity: Normal Keywords: ff68-esr, tbb-9.0-must-nightly, TorBrowserTeam201908R
Cc: tbb-team, acat Actual Points:
Parent ID: Points:
Reviewer: Sponsor: Sponsor44-can

Description

In addition to removal of overlays (#29197), we must make other changes in Tor Launcher for ESR68 compatibility. For example, most of the "on" event attributes within XUL wizard elements have been replaced with events.

Child Tickets

Attachments (3)

current.png (39.7 KB) - added by acat 2 months ago.
patch.png (32.8 KB) - added by acat 2 months ago.
patch2.png (34.9 KB) - added by acat 2 months ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 3 months ago by mcs

Keywords: TorBrowserTeam201907R added; TorBrowserTeam201907 removed
Status: assignedneeds_review

Here is a patch for review; please read the commit message for more details about what we changed and why:
https://gitweb.torproject.org/user/brade/tor-launcher.git/commit/?h=bug29197-01&id=cf26f0d0bd5b1e225ca0dd1c95c8f1c0d33663ea

This patch is on our bug29197-01 branch since it would be best to test this fix with the one for that ticket. So far Kathy and I have only tested on macOS with a non-rbm-based Tor Browser build.

comment:2 Changed 3 months ago by mcs

Summary: modify Tor Launcher so it is compatible with ESR68Modify Tor Launcher so it is compatible with ESR68

comment:3 Changed 3 months ago by gk

Cc: acat added

acat: could you review and test that patch as well?

comment:4 Changed 2 months ago by gk

Keywords: TorBrowserTeam201908R added; TorBrowserTeam201907R removed

No July any longer.

Changed 2 months ago by acat

Attachment: current.png added

Changed 2 months ago by acat

Attachment: patch.png added

comment:5 Changed 2 months ago by acat

Mostly looks good and works fine for me.

One small question I have regarding the added css:

+groupbox {
+  margin-left: 16px;
+  padding: 8px;
+  background-color: rgba(215, 215, 219, 0.5);

Where does the background-color come from? I checked in my machine with current tor-launcher, and I don't see any background-color difference with the background, but after this patch I do (see screenshots).

comment:6 Changed 2 months ago by acat

I also tested the Request bridge option (had to add bridges to the prefs, otherwise the section does not appear). It fails with Tor unexpectedly closed for me, but it might be an issue that will not reproduce with a properly built nightly, so I'm not sure (I copied the esr68 Firefox built files over a release tor-browser folder).

comment:7 in reply to:  5 ; Changed 2 months ago by mcs

Replying to acat:

Mostly looks good and works fine for me.

One small question I have regarding the added css:

+groupbox {
+  margin-left: 16px;
+  padding: 8px;
+  background-color: rgba(215, 215, 219, 0.5);

Where does the background-color come from? I checked in my machine with current tor-launcher, and I don't see any background-color difference with the background, but after this patch I do (see screenshots).

In ESR60, OS styling was used for groupbox. Kathy and I did not realize that it is so different on Linux vs. macOS vs. Windows... we just did something that is similar to the older macOS look. It looks like on Linux there was no styling to "set off" the groupbox contents, while macOS had a darkened background, and Windows 10 had a gray line around the contents. I like the shading/darkened background but we can revisit this later once more people can look at it, e.g., in a nightly build.

Last edited 2 months ago by mcs (previous) (diff)

comment:8 in reply to:  6 Changed 2 months ago by mcs

Replying to acat:

I also tested the Request bridge option (had to add bridges to the prefs, otherwise the section does not appear). It fails with Tor unexpectedly closed for me, but it might be an issue that will not reproduce with a properly built nightly, so I'm not sure (I copied the esr68 Firefox built files over a release tor-browser folder).

I forgot to mention that moat (aka "Request a bridge") is currently broken due to meek-client-torbrowser's use of an XPCOM extension. Our plan to avoid that problem is #29430, although there is also #29347. I added keywords to #29430.

comment:9 in reply to:  7 Changed 2 months ago by mcs

Keywords: TorBrowserTeam201908 added; TorBrowserTeam201908R removed
Status: needs_reviewneeds_revision

Replying to mcs:

In ESR60, OS styling was used for groupbox. Kathy and I did not realize that it is so different on Linux vs. macOS vs. Windows... we just did something that is similar to the older macOS look. It looks like on Linux there was no styling to "set off" the groupbox contents, while macOS had a darkened background, and Windows 10 had a gray line around the contents. I like the shading/darkened background but we can revisit this later once more people can look at it, e.g., in a nightly build.

I take back some of what I said. I just looked at your patch.png screnshot and it is not pretty. Maybe we can replace the background color with a simple line border that matches the text color. But if we cannot figure out how to do that, we will just remove the shading and not replace it with anything.

comment:10 Changed 2 months ago by mcs

Keywords: TorBrowserTeam201908R added; TorBrowserTeam201908 removed
Status: needs_revisionneeds_review

Here is a revised patch:
https://gitweb.torproject.org/user/brade/tor-launcher.git/commit/?h=bug31300-02&id=1af54380f289426fea9dfa24c4fd6cf171046723

The only difference is that we replaced the groupbox background-color style with a 1px GrayText border. Alex, it would be great if you could test this on your Linux system to see how it looks.

Changed 2 months ago by acat

Attachment: patch2.png added

comment:11 Changed 2 months ago by acat

I think it looks better (see patch2.png). Good enough for the nightly for sure :)

In any case, as you said we can refine later.

comment:12 Changed 2 months ago by pili

Sponsor: Sponsor44-can

Adding Sponsor 44 to ESR68 tickets

comment:13 Changed 2 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Thanks! Merged to master (commit 1af54380f289426fea9dfa24c4fd6cf171046723).

comment:14 Changed 8 weeks ago by mcs

A follow up ticket for the moat client code: #31487.

Note: See TracTickets for help on using tickets.