Details

Reviewers

• abosh
tomek
• jacek

Commits

rCOMM9c0cb22a723c: [web] Remove `safari-pinned-tab.svg`

Summary

Addresses https://linear.app/comm/issue/ENG-963/remove-old-squad-cal-favicon-on-safari

It used to be that Safari didn't display favicons for each tab and instead showed the page title. However, if a tab was pinned--or there wasn't enough space to show an abbreviated title--it would display the first letter of the page title:

reopen-historical-tabs-mac-safari.jpg (880×1 px, 175 KB)

Save-all-open-Safari-tabs-in-bookmark-folder.jpg (349×620 px, 28 KB)

Apple gave the option of providing a black square SVG (safari-pinned-tab.svg) to be displayed for "pinned" tabs and the ability to specify a tint color for "active" tabs.

However, recent versions of Safari display favicons for tabs across the board (it's no longer even possible to disable them), and including safari-pinned-tab.svg is no longer necessary... but Safari will still use it if it's provided. Based on what I've read, it seems like the only reason to specify safari-pinned-tab.svg is if you want something other than the default favicon to be displayed on just Safari. I don't think that's something we care about.

Test Plan

Ensured correct favicon displayed in Safari/Chrome after clearing cache.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Jun 24 2022, 3:14 PM

Herald added subscribers: • adrian, ashoat. · View Herald TranscriptJun 24 2022, 3:14 PM

atul edited the summary of this revision. (Show Details)Jun 24 2022, 3:16 PM

Harbormaster completed remote builds in B9969: Diff 13789.Jun 24 2022, 3:19 PM

atul requested review of this revision.Jun 24 2022, 3:19 PM

see if tunnelbroker CI gets triggered

Harbormaster completed remote builds in B9974: Diff 13790.Jun 24 2022, 7:20 PM

trigger ci

Harbormaster completed remote builds in B9975: Diff 13791.Jun 24 2022, 10:15 PM

trigger CI

Harbormaster completed remote builds in B9976: Diff 13792.Jun 25 2022, 12:26 AM

trigger ci

Harbormaster completed remote builds in B9977: Diff 13793.Jun 25 2022, 12:31 AM

Harbormaster completed remote builds in B9978: Diff 13794.

trigger ci

Harbormaster completed remote builds in B9979: Diff 13795.Jun 25 2022, 12:37 AM

trigger ci

Harbormaster completed remote builds in B9980: Diff 13796.Jun 25 2022, 12:41 AM

Harbormaster completed remote builds in B9981: Diff 13797.Jun 25 2022, 12:45 AM

Harbormaster completed remote builds in B9982: Diff 13798.Jun 25 2022, 12:49 AM

trigger

Harbormaster completed remote builds in B9983: Diff 13800.Jun 25 2022, 11:22 AM

trigger

Harbormaster completed remote builds in B9984: Diff 13801.Jun 25 2022, 11:26 AM

trigger

Harbormaster completed remote builds in B9985: Diff 13802.Jun 25 2022, 11:30 AM

Harbormaster completed remote builds in B9986: Diff 13803.Jun 25 2022, 11:35 AM

Harbormaster completed remote builds in B9987: Diff 13804.Jun 25 2022, 11:39 AM

Harbormaster completed remote builds in B9988: Diff 13805.Jun 25 2022, 11:43 AM

Harbormaster completed remote builds in B9989: Diff 13806.Jun 25 2022, 11:48 AM

Harbormaster completed remote builds in B9990: Diff 13807.Jun 25 2022, 11:52 AM

tomek accepted this revision.Jun 27 2022, 2:37 AM

This revision is now accepted and ready to land.Jun 27 2022, 2:37 AM

However, recent versions of Safari display favicons for tabs across the board (it's no longer even possible to disable them), and including safari-pinned-tab.svg is no longer necessary... but Safari will still use it if it's provided. Based on what I've read, it seems like the only reason to specify safari-pinned-tab.svg is if you want something other than the default favicon to be displayed on just Safari. I don't think that's something we care about.

Got it. That makes sense.

Was able to reproduce this diff with the intended effects on Safari, so looks good to me! One last thing, and this is probably better suited for a future diff, but it seems like the Comm icon on Safari isn't trimmed perfectly/has white bezels: