Opened 6 years ago

Closed 5 years ago

#10751 closed enhancement (fixed)

Adapt Torbutton to the Australis UI

Reported by: gk Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Keywords: TorBrowserTeam201409, MikePerry201409R, tbb-usability, ff31-esr, tbb-torbutton
Cc: intrigeri, brade, mcs, arthuredelstein@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Assuming Australis hits stable when we are dealing with a TorBrowser based on Fx 31 we should streamline the Torbutton appearance in the toolbar. There are some rough edges due to the new UI paradigm.

Child Tickets

Attachments (2)

0001-Bug-10751.1-Load-HUDService-only-if-it-s-not-already.patch (1.9 KB) - added by arthuredelstein 5 years ago.
0002-Bug-10751.2-Adapt-Torbutton-to-Australis-UI.patch (2.2 KB) - added by arthuredelstein 5 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 5 years ago by intrigeri

Cc: intrigeri added

comment:2 Changed 5 years ago by mcs

Cc: brade mcs added

This is also an opportunity to review the Torbutton UI and think about how to streamline it and make it better. Related bugs: #10632, #11175, #12514 (and others, I am sure)

comment:3 Changed 5 years ago by erinn

Component: TorBrowserButtonTor Browser
Keywords: tbb-torbutton added
Owner: changed from mikeperry to tbb-team

comment:4 Changed 5 years ago by arthuredelstein

Cc: arthuredelstein@… added

comment:5 Changed 5 years ago by arthuredelstein

I have a branch at https://github.com/arthuredelstein/torbutton/commits/12620 that contains fixes for making Torbutton compatible with the esr31-port-untested build at https://github.com/arthuredelstein/tor-browser/commits/esr31-port-untested. (See also #12620)

Last edited 5 years ago by arthuredelstein (previous) (diff)

comment:6 Changed 5 years ago by mikeperry

Ideally, if these are all the changes we need for FF31esr, this new Torbutton would still work on FF24esr, so we don't have to make a separate branch for the 31esr series.

Is it possible to wrap that HUDService import in a try/catch or version check (as is done in torbutton_init())?

comment:7 Changed 5 years ago by arthuredelstein

Yes, I think we can do this.

comment:8 Changed 5 years ago by arthuredelstein

I've updated the branch to handle both ESR24 and ESR31 cases. I'm still working on it, in part because there's a weird bug with the popup menu where it only pops up on the second run of TBB.

In the medium term, it might be nice to fully adapt to the Australis style of popup panel. That probably means breaking backward compatibility, so perhaps that should wait until the ESR24 version is retired.

comment:9 Changed 5 years ago by arthuredelstein

OK, I think it's fixed now. The popup menu now works on first run.

comment:10 in reply to:  6 ; Changed 5 years ago by gk

Replying to mikeperry:

Ideally, if these are all the changes we need for FF31esr, this new Torbutton would still work on FF24esr, so we don't have to make a separate branch for the 31esr series.

#10716 makes the ESR 31 Torbutton incompatible with the ESR 24 one unless we do some version detection within Torbutton and or just ship both the old and the new logic (as the current patch does), a thing I'd like to avoid.

comment:11 in reply to:  10 Changed 5 years ago by arthuredelstein

Replying to gk:

Replying to mikeperry:

Ideally, if these are all the changes we need for FF31esr, this new Torbutton would still work on FF24esr, so we don't have to make a separate branch for the 31esr series.

#10716 makes the ESR 31 Torbutton incompatible with the ESR 24 one unless we do some version detection within Torbutton and or just ship both the old and the new logic (as the current patch does), a thing I'd like to avoid.

I can see good arguments both for and against using version detection. We could use version detection during the transition period bewteen ESRs and then delete code for ESR 24 after it's retired. What do you think? I'll post patches here for review once we've decided which way to go.

comment:12 Changed 5 years ago by gk

As this is affects only a short transition period and no huge changes seem to be involved I think having just one branch + version detection might be less overhead, yes.

Changed 5 years ago by arthuredelstein

comment:13 Changed 5 years ago by arthuredelstein

Status: newneeds_review

Here's the patches for review. I've tested them in the latest version of TBB and the ESR31 branch at #12620.

comment:14 Changed 5 years ago by arthuredelstein

Keywords: TorBrowserTeam201408 MikePerry201408Rtbb-usability added; tbb-usability removed

comment:15 Changed 5 years ago by arthuredelstein

Keywords: MikePerry201408R tbb-usability added; MikePerry201408Rtbb-usability removed

comment:16 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201409 MikePerry201409R added; TorBrowserTeam201408 MikePerry201408R removed

comment:17 Changed 5 years ago by gk

The patches look good to me.

comment:18 Changed 5 years ago by mikeperry

Resolution: fixed
Status: needs_reviewclosed

Yep. Merged to origin/master.

Note: See TracTickets for help on using tickets.