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

Automatically clear data upon quitting #2600

Merged
merged 26 commits into from
Apr 28, 2024
Merged

Conversation

tomasstrba
Copy link
Contributor

@tomasstrba tomasstrba commented Apr 11, 2024

Task/Issue URL: https://app.asana.com/0/1177771139624306/1205062321200340/f

Description:
Adding Burn on Quit feature for macOS. Upon standard exit, data are cleared and the fire animation is presented. If macOS is restarting, the app delays the restart until all data have been cleared. In the event of an unexpected termination, such as a crash or force quit, data clearing is handled at the next startup.

Steps to test this PR:
Test standard application termination with the burn on quit disabled and the session restoration disabled

  1. Run the app
  2. Open General Settings and set "Open a new window" in "On Startup" section
  3. Open bunch of tabs and visit few sites
  4. Quit the app
  5. Run the app again and verify no tabs are restored and the history from the previous session is present

Test standard application termination with the burn on quit disabled and the session restoration enabled

  1. Run the app
  2. Open General Settings and set "Reopen all windows from last session" in "On Startup" section
  3. Open bunch of tabs and visit few sites
  4. Quit the app
  5. Run the app again and verify all tabs are restored and the history from the previous session is present

Test standard application termination with the 'burn on quit' enabled and the 'warn before quit' disabled

  1. Run the app
  2. Open Data Clearing Settings and check "Clear Data upon Quitting"
  3. Open bunch of tabs and visit few sites
  4. Quit the app
  5. Verify the burning animation is present
  6. Run the app again and verify all data including tabs are gone.

Test standard application termination with the 'burn on quit' enabled and the 'warn before quit' enabled

  1. Run the app
  2. Open Data Clearing Settings, verify "Clear Data upon Quitting" is checked and additionally check also 'warn before quit'
  3. Open bunch of tabs and visit few sites
  4. Try to quit the app
  5. Make sure the confirmation dialog is presented
  6. Click on cancel and verify the browser session continues. (The app didn't terminate)
  7. Try to the app again and confirm the dialog.
  8. Verify the burning animation is present
  9. Run the app again and verify all data including tabs are gone.

Test unexpected application termination with the 'burn on quit' enabled

  1. Run the app
  2. Open Data Clearing Settings and verify "Clear Data upon Quitting" is checked
  3. Open bunch of tabs and visit few sites
  4. Force quit the app
  5. Run the app again
  6. Verify the burning animation is present and all data including tabs from previous session are gone.

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

@tomasstrba tomasstrba requested a review from mallexxx April 11, 2024 09:15
@github-actions github-actions bot added the bot: not in app board Added by automation for pull requests with tasks not added to macOS App Board Asana project label Apr 12, 2024
@tomasstrba tomasstrba removed the bot: not in app board Added by automation for pull requests with tasks not added to macOS App Board Asana project label Apr 12, 2024
@duckduckgo duckduckgo deleted a comment from github-actions bot Apr 12, 2024
Copy link
Contributor

github-actions bot commented Apr 12, 2024

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

Generated by 🚫 dangerJS against 964cbe9

@mallexxx mallexxx self-assigned this Apr 17, 2024
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<Scheme
LastUpgradeVersion = "1530"
version = "1.8">
version = "1.7">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Xcode 15.3 artifact, roll back please

static func disableAutoClearToEnableSessionRestore() -> String {
let localized = NSLocalizedString("disable.auto.clear.to.enable.session.restore",
value: "Disable the %@ setting to enable session restore on startup.",
comment: "Information label in Settings. It tells user that to enable session restoration setting they have to disable burn on quit")
Copy link
Collaborator

@mallexxx mallexxx Apr 17, 2024

Choose a reason for hiding this comment

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

add a comment for translator that %@ will be replaced with the "Auto-Clear" setting name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed %@ and used the same approach you suggested for showDataClearingSettings

let localized = NSLocalizedString("show.data.clearing.settings",
value: "Show %@ Settings.",
comment: "Button in Settings. It navigates user to Data Clearing Settings.")
return String(format: localized, dataClearing)
Copy link
Collaborator

@mallexxx mallexxx Apr 17, 2024

Choose a reason for hiding this comment

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

the string format seems misaligned here as it doesn‘t consider declension of nouns ("Очистка данных" -> "Открыть настройки очистк-и- данных"), I think it‘s better to set the whole string for locatizations.

Copy link
Collaborator

@mallexxx mallexxx left a comment

Choose a reason for hiding this comment

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

Looks good, @tomasstrba!
Validated data clearing working on quit and on launch after termination.
See minor comments inline

@mallexxx mallexxx assigned tomasstrba and unassigned mallexxx Apr 17, 2024
@tomasstrba tomasstrba merged commit 36aa51f into main Apr 28, 2024
14 checks passed
@tomasstrba tomasstrba deleted the tom/automatically-clear-data branch April 28, 2024 19:28
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