Opened 13 months ago

Closed 13 months ago

Last modified 12 months ago

#32097 closed defect (fixed)

Fix conflicts in mobile onbarding while rebasing to esr68.2.0

Reported by: gk Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: TorBrowserTeam201910R, tbb-9.0-must
Cc: sysrqb Actual Points: 0.5
Parent ID: Points: 0.5
Reviewer: Sponsor:


It turns out that there are a number of conflicts between our mobile onboarding code and the coming esr68.2.0 which we need to think about how to resolve. They stem in particular from


Not sure whether we want to just back out those changes or make them otherwise compatible with our modifications.

Child Tickets

Attachments (1)

bug32097_merge_conflict_resolution.patch (10.9 KB) - added by sysrqb 13 months ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 13 months ago by sysrqb

I pushed bug32097_00 which is tor-browser-68.1.0esr-9.0-2-build4 rebased onto gecko-dev/esr68 (Mozilla's github esr68 branch). Would you prefer this as a fixup! commit? I'm not sure the best way of solving this.

For reference, the only conflict is on Bug 25696 - Implement alpha onboarding for Tor Browser for Android and the diff with resolved conflicts is:

diff -u <(git show 3b3dbd5478641efe578054300018ec81935d4f2c) <(git show 369ed1ff2eb167eb2555125e73a05d27689c5584)

(I don't know why we have minor changes in the strings files.)

Changed 13 months ago by sysrqb

comment:2 Changed 13 months ago by sysrqb

hrm. The build fails with that branch - proguard symbols.

comment:3 Changed 13 months ago by sysrqb

Okay, I clobbered and rebuilt, and now it is happy. Unfortunately, now we have a "Sign Up" button on the botton of all onboarding panels and the Activity Stream (#31983) screen has a very large "Sign Up" button at the top, too. I'll fix those.

comment:4 Changed 13 months ago by sysrqb

Keywords: TorBrowserTeam201910R added; TorBrowserTeam201910 removed
Status: newneeds_review

I used 22182890f20069201b37a0e8d9a849d2e54fcc25 as the new base. I reverted (in order): 484ec9407da4234000a73d2426f09de62de0274e ab8797462cefdaf99d10e1981f2a168750cc697a 303c1ef548d23dfd0d8aa1e9e61481c1baea02a8 896fa0df588de10b22e827b8f9999c69daa9e05d 1c54037a5bb7ded841b5a795041772a4571944f9 a7cc5967877636728e29ea865dc46a2889b7de58 a3f466a3ac73f0d11f3f3ba054173087e66a0ab5 and then rebased origin/tor-browser-68.1.0esr-9.0-3 onto the result.

I fear we may have more problems with this in the future if/when Mozilla modify the onboarding panels again. I felt resolving the merge conflict wasn't easy to review, but we can reconsider this if needed.

I pushed branch bug32097_02 based on 68.2.0esr.

comment:5 in reply to:  4 Changed 13 months ago by sysrqb

Replying to sysrqb:

I used 22182890f20069201b37a0e8d9a849d2e54fcc25 as the new base. I reverted (in order):

Sorry, it's based on 8b37b0197138897a766056fe3d4e6c53b891cdcb, branch bug32097_02.

comment:6 Changed 13 months ago by gk

Status: needs_reviewneeds_information

Okay, I went ahead in the morning and committed the 7 backouts to get the build going but I am still trying to understand why those 7 patches got chosen given that there are a bunch of more bugs involved.

For instance, the patch for got backed out, which seems right to me. However, the regression fix for that bug, tracked in did not. So, we are now actually using code that was only meant to get used with a fix we backed out... Are we good with that?

comment:7 Changed 13 months ago by sysrqb

Status: needs_informationneeds_review

I began with only resolving merge conflicts. That got me most of the way to creating a clean build. Then I noticed the new patches added new Sync "Sign Up" buttons on the onboarding panels and Activity Stream panel. I wrote fixup patches for deleting the button on the onboarding panels and I backed out the commit for the Activity Stream. Unfortunately, this resulted in a build failure due to undefined variables resulting from the backed out patch. At this point I decided I'd try backing out all the patches needed for successfully building 68.2.0. This is why I didn't backout all commits, like Bug 1586770. The patch for that bug didn't break any functionality or introduce conflicting code changes.

I looked at the patch and we can keep it. I'll look through the other Fennec patches for 68.2.0, as well, and confirm we didn't miss anything.

comment:8 Changed 13 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Okay, makes sense. Thanks for the explanation. The commits on tor-browser-68.2.0esr-9.0-1 and tor-browser-68.2.0esr-9.5-1 are

2ba949f9ba65ba71c9805635a904a68488404186 c7c1d14e1627aebb749dd5e0065e49c1436530d3
d05b3c8d35ac51311a8454e3e26edacbc45e3ce4 e3af40f4b3b7aecf53a2442ad3417055b87719a2
ec8b0640ed1a383b093dd044f95fdf9934070046 15c908b1c374b667acbf8711954b032b0bb4d8f8

We are done here. If there is anything else to fix up then please open a new ticket.

comment:9 Changed 12 months ago by sysrqb

Actual Points: 0.5
Note: See TracTickets for help on using tickets.