Opened 4 years ago

Closed 4 years ago

#16728 closed enhancement (fixed)

Test cases for favicon isolation

Reported by: mikeperry Owned by: boklm
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-testcase, TorBrowserTeam201602
Cc: arthuredelstein Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

We need either unittests or integration tests for #16448

Child Tickets

Change History (5)

comment:1 Changed 4 years ago by arthuredelstein

Severity: Normal
Summary: Test cases for favicon and OCSP isolationTest cases for favicon isolation

(I am moving the OCSP part of this ticket to #18334, as they are separate tasks.)

Version 0, edited 4 years ago by arthuredelstein (next)

comment:2 Changed 4 years ago by arthuredelstein

Keywords: TorBrowserTeam201602R added
Status: newneeds_review

Here's a patch that adds tests for favicon first-party isolation to our existing regression tests for first-party isolation of cache (#13749.2).

https://github.com/arthuredelstein/tor-browser/commit/16728

(The original favicon isolation patch is #13670.1.)

comment:3 Changed 4 years ago by gk

Keywords: TorBrowserTeam201602 added; TorBrowserTeam201602R removed
Status: needs_reviewneeds_revision

Looks good. Just some nits: There are at least two comments that need to get updated including the favicon test as well (line 7/8 and line 107 in browser_cacheFirstParty.js). While you are trying to get rid of superfluous whitespaces, at the end of line 113 in browser_cacheFirstParty.js is another one.

comment:4 in reply to:  3 Changed 4 years ago by arthuredelstein

Replying to gk:

Looks good. Just some nits: There are at least two comments that need to get updated including the favicon test as well (line 7/8 and line 107 in browser_cacheFirstParty.js). While you are trying to get rid of superfluous whitespaces, at the end of line 113 in browser_cacheFirstParty.js is another one.

Thanks for the review. Here's a revised version with the changes you suggested.

https://github.com/arthuredelstein/tor-browser/commit/16728+1

For the record, I tried reverting our favicon patch (#13670.1) and this patch correctly detected the failure to isolate the favicons by URL bar domain.

comment:5 Changed 4 years ago by gk

Resolution: fixed
Status: needs_revisionclosed

Thanks. This is commit df2cdeb77b7133ea672920caa8c2eac5d69f71c6 on tor-browser-38.6.1esr-6.0-1.

Note: See TracTickets for help on using tickets.