Opened 8 months ago

Last modified 8 months ago

#22002 new defect

Backport Mozilla's SVG-disabling patch

Reported by: arthuredelstein Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: ff52-esr
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Mozilla partially uplifted our SVG disabling patch, but a lot of edge cases are left out that we need to add in order to use the uplifted patch.

Child Tickets

Change History (7)

comment:1 Changed 8 months ago by arthuredelstein

Keywords: tbb-security added
Summary: Backport the SVG patchBackport Mozilla's SVG-disabling patch

comment:2 Changed 8 months ago by gk

Why do we want to backport that patch if things are missing and we have a working patch in our tree?

comment:3 Changed 8 months ago by gk

Oh, and where is the security angle wrt this backport?

comment:4 in reply to:  2 ; Changed 8 months ago by arthuredelstein

Replying to gk:

Why do we want to backport that patch if things are missing and we have a working patch in our tree?

We don't have to necessarily. Because the backport seemed to fix the crash, I was attempting to work with it, but found several missing edges cases. That's why I suggested in #21962 that we stick with the working patch, with Mark and Kathy's fixup.

In the long run, given that we hope to reduce our maintenance by uplifting our SVG-disabling code, it seems to make sense that we would backport the Mozilla patch and add back the missing parts. Or we could ask Mozilla to work on that.

FWIW, here is my WIP branch. I'm going to leave it at this stage until we decide on what to do.
https://github.com/arthuredelstein/tor-browser/commits/22002_wip

comment:5 in reply to:  3 Changed 8 months ago by cypherpunks

Replying to gk:

Oh, and where is the security angle wrt this backport?

Part of your security slider ;)

(Do you think any of your patches can cope with e10s (edge cases)?)

comment:6 Changed 8 months ago by arthuredelstein

Keywords: ff52-esr added; tbb-security removed

comment:7 in reply to:  4 Changed 8 months ago by gk

Replying to arthuredelstein:

Replying to gk:

Why do we want to backport that patch if things are missing and we have a working patch in our tree?

We don't have to necessarily. Because the backport seemed to fix the crash, I was attempting to work with it, but found several missing edges cases. That's why I suggested in #21962 that we stick with the working patch, with Mark and Kathy's fixup.

In the long run, given that we hope to reduce our maintenance by uplifting our SVG-disabling code, it seems to make sense that we would backport the Mozilla patch and add back the missing parts. Or we could ask Mozilla to work on that.

Asking Mozilla to add the missing pieces seems like a good idea to me as I guess they are more familiar with the way the patch on m-c is written. Adding the remaining things should be faster that way.

Note: See TracTickets for help on using tickets.