Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove widget favicon coupling and move favicon code from core to the app #3590

Merged
merged 4 commits into from
Nov 21, 2024

Conversation

brindy
Copy link
Contributor

@brindy brindy commented Nov 18, 2024

Task/Issue URL: https://app.asana.com/0/414709148257752/1208785980508806/f
Tech Design URL:
CC:

Description:
Moves the favicon code the app bundle to make future refactoring easier.

Includes refactoring from #3593

Steps to test this PR:

  1. Run an old version of the app and add some favorites
  2. Add the favorites widget and validate favicons appear
  3. Upgrade to this version of the app
  4. Open some tabs and ensure that the favicons appear in the tab switcher.
  5. Check the cache location for the images. Look for favicons tabs location in Xcode console and go to the parent of that directory.
  6. Also check the favicons fireproof location
  7. Use the fire button.
  8. Ensure that images are removed from the tabs location but not from the fireproof location
  9. Repeat with a clean install

@brindy brindy requested a review from bwaresiak November 18, 2024 16:45
Copy link
Collaborator

@bwaresiak bwaresiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, it's much cleaner, caches work and clean as expected. 🎉

…le (#3593)

Task/Issue URL:
https://app.asana.com/0/392891325557410/1208796954308702/f
Tech Design URL:
CC: @bwaresiak 

**Description**:
Rename preserved logins to fireproofing and use protocol where possible

This is stacked on #3590 - am
trying to make baby steps towards removing a few of the gnarlier
singletons (Favicons and Fireproofing at least)

**Steps to test this PR**:
1. Smoke test fireproofing with cookies, you can use
https://www.setcookie.net (remember to set an expiry date for it to
work)
2. Go into Settings > Data Clearing > Fireproof sites and make sure fire
proofed sites are visible and can be deleted from there
3. Check it works and visible in the settings UI
Copy link

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.
Messages
📖

You seem to be updating localized strings. Make sure that you request translations and include translated strings before you ship your change. See Localization Guidelines for more information.

Generated by 🚫 dangerJS against c33a7cd

@brindy brindy merged commit f5f0a13 into main Nov 21, 2024
18 checks passed
@brindy brindy deleted the brindy/remove-widget-favicon-coupling branch November 21, 2024 12:18
samsymons added a commit that referenced this pull request Nov 22, 2024
…g-error

# By Dominik Kapusta (3) and others
# Via Dominik Kapusta (1) and others
* main:
  Expand persistent pixel usage (#3596)
  Release 7.146.0-2 (#3610)
  Bump BSK to 211.1.1 (#3609)
  Update BSK with a C-S-S fix (#3608)
  remove widget favicon coupling and move favicon code from core to the app (#3590)
  Update Content Scope Scripts to 6.38.0 (#3605)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants