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

Improve Safari favorite importing #436

Merged
merged 19 commits into from
Feb 24, 2022

Conversation

samsymons
Copy link
Collaborator

Task/Issue URL: https://app.asana.com/0/1177771139624306/1201796758953738/f
Tech Design URL:
CC:

Description:

This PR makes a few changes:

  1. A new Reset Bookmarks option has been added to the Debug menu - happy testing
  2. Bookmarks are now sorted from oldest to newest, matching how other browsers work
  3. Safari favorites are correctly marked when importing
  4. The resulting folder structure now depends on which browser you import from. Safari imports all bookmarks to the root level, except favorites, whereas others import all bookmarks to the root level except for their "other bookmarks" folders.

The end goal of this change was to make the imported bookmark structure match the source browser as closely as possible. This isn't always possible - for instance, Chromium browsers don't have the concept of a favorite bookmark - but we've tried to get closer.

Steps to test this PR:

  1. Import bookmarks from all supported browsers and verify that bookmarks are in the same order that they are in the browser you imported from

Testing checklist:

  • Test with Release configuration

Internal references:

Software Engineering Expectations
Technical Design Template
When ready for review, remember to post the PR in MM

* develop:
  Update Fire Popover graphics (#426)
  Refresh the address bar when reloading (#413)
  Fix tabs leakage after Drag-Drop (#423)
# By Alexey Martemyanov (2) and others
# Via GitHub (1) and Tomas Strba (1)
* develop:
  Fix crash on SecureVault corruption (#417)
  ensure app usage is sent (#429)
  Fix non-debug builds (#428)
  new tds url (#430)
  Sparkle 1.27.1 (#411)
  Disable CVDisplayLing logging (#421)
  Version 0.18.7

# Conflicts:
#	DuckDuckGo/Menus/MainMenuActions.swift
* develop:
  Set BrowserServicesKit to 8.0.2.
* develop:
  Waiting for ContentBlockingRules to be applied before navigation (#402)
  Cookie prompt management (#312)
  Pass config data to Autofill UserScript (#418)
  Fix video fullscreen exit crash (#295)
return
}
topFavorites = Array(favorites.prefix(Constants.maxNumberOfFavorites).reversed())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We now import bookmarks and display them in a consistent order, so they don't need to be reversed on the Home Screen.

@brindy brindy self-assigned this Feb 23, 2022
Copy link
Contributor

@brindy brindy left a comment

Choose a reason for hiding this comment

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

LGTM! Just a couple of suggestions then feel free to merge.

DuckDuckGo/Bookmarks/Services/BookmarkStore.swift Outdated Show resolved Hide resolved
DuckDuckGo/Bookmarks/Services/BookmarkStore.swift Outdated Show resolved Hide resolved
DuckDuckGo/Data Import/Bookmarks/BookmarkImport.swift Outdated Show resolved Hide resolved
@brindy brindy assigned samsymons and unassigned brindy Feb 23, 2022
* develop:
  Logins+ filtering and sorting (#427)
  Remove the Never option from the Save Credentials VC. (#437)
  Check the current sheets for a lock screen before presenting. (#434)
  macOS 12 crash report handling (#431)
@samsymons samsymons merged commit 873a1ce into develop Feb 24, 2022
@samsymons samsymons deleted the sam/import-safari-favorites-correctly branch February 24, 2022 05:54
jonathanKingston pushed a commit that referenced this pull request Feb 25, 2022
Task/Issue URL: https://app.asana.com/0/1177771139624306/1201796758953738/f
Tech Design URL:
CC:

Description:

This PR makes a few changes:

A new Reset Bookmarks option has been added to the Debug menu - happy testing
Bookmarks are now sorted from oldest to newest, matching how other browsers work
Safari favorites are correctly marked when importing
The resulting folder structure now depends on which browser you import from. Safari imports all bookmarks to the root level, except favorites, whereas others import all bookmarks to the root level except for their "other bookmarks" folders.
The end goal of this change was to make the imported bookmark structure match the source browser as closely as possible. This isn't always possible - for instance, Chromium browsers don't have the concept of a favorite bookmark - but we've tried to get closer.

Steps to test this PR:

Import bookmarks from all supported browsers and verify that bookmarks are in the same order that they are in the browser you imported from
samsymons added a commit that referenced this pull request Feb 25, 2022
# By Sam Macbeth (2) and others
# Via GitHub
* develop:
  Top autofill (#432)
  Option to add new notes or edit existing is disabled (#446)
  Use our own autoconsent fork (#444)
  New Feedback Form (#424)
  Update privacy dashboard (#440)
  Fix crash when background tabs trigger cookie popup (#439)
  Update clickToLoadConfig.json (#435)
  rename weakAssign to assign(to:onWeaklyHeld:) (#442)
  Improve Safari favorite importing (#436)

# 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