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

Bugfix FXIOS-9407: Open deeplinks in correct browsing mode #20927

Conversation

MattLichtenstein
Copy link
Collaborator

@MattLichtenstein MattLichtenstein commented Jul 8, 2024

📜 Tickets

Jira ticket
Github issue

💡 Description

This bug essentially states that all links opened from outside the apps main target will open in regular browsing mode. Although all private browsing data is cleared when the app is killed, these paths may not always be initiated while the app is not in memory, and therefore, could happen while the app is currently in private browsing (via a private selectedTab). To prevent the user from accidentally switching browsing modes, the following changes were made:

  • The “Go to copied link” widget action will now open the copied link (search query or URL) in the current browsing mode
  • The “Open last bookmark” home screen quick action will now open the most recently bookmarked URL in the current browsing mode
  • The "Open in Firefox" share sheet action will now open the target URL in the current browsing mode
  • The "Top sites" widget action will now open the selected URL in the current borwsing mode

Also fixed was a bug where search queries (saved in the pasteboard as strings instead or URL's, even if they are well-formed URL's) opened via “Go to copied link” widget would navigate to the respective web page before the newly created tab was selected, causing the app to display a blank/homescreen tab to the user, while the navigation intended to take place on the new tab, happened in the previously open tab

Notes:

  • We handle opening pasteboard text vs pasteboard URL's a bit different. When using the "Go to copied link" widget or "Open in Firefox" share sheet action, opening a URL from the pasteboard, we will first search for a tab already containing that URL and switch to it if possible (preferring regular browsing over private if the URL is open in both browsing modes), while opening text from the pasteboard will always open in a new tab, even if it is formed as a URL that is already open in another tab.
    • The "Open last bookmark" follows the same policy as pasteboard URL's mentioned above in that it will not create a new tab if the URL is already open in at least one of the browsing modes, and prefers regular over private.

Testing:

To test these changes, ensure all links opened in Firefox from outside the app open in Firefoxes current browsing mode (regular or private). This includes:

  • The "Go to copied link" widget action
  • The "Open last bookmark" home screen quick action"
  • The "Open in Firefox" share sheet action
  • The "Load in background" share sheet action

📝 Checklist

You have to check all boxes before merging

  • Filled in the above information (tickets numbers and description of your work)
  • Updated the PR name to follow our PR naming guidelines
  • Wrote unit tests and/or ensured the tests suite is passing
  • When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
  • If needed, I updated documentation / comments for complex code and public methods
  • If needed, added a backport comment (example @Mergifyio backport release/v120)

Sorry, something went wrong.

Copy link
Contributor

mergify bot commented Jul 11, 2024

This pull request has conflicts when rebasing. Could you fix it @MattLichtenstein? 🙏

@MattLichtenstein MattLichtenstein force-pushed the FXIOS-9407_Open-Deeplinks-In-Correct-Browsing-Mode branch from 1863eba to 038bbea Compare July 11, 2024 19:41
@MattLichtenstein MattLichtenstein marked this pull request as ready for review July 11, 2024 19:41
@MattLichtenstein MattLichtenstein requested a review from a team as a code owner July 11, 2024 19:41
@MattLichtenstein MattLichtenstein marked this pull request as draft July 12, 2024 15:03
@MattLichtenstein MattLichtenstein marked this pull request as ready for review July 12, 2024 15:03
Copy link
Contributor

mergify bot commented Jul 15, 2024

This pull request has conflicts when rebasing. Could you fix it @MattLichtenstein? 🙏

@MattLichtenstein MattLichtenstein force-pushed the FXIOS-9407_Open-Deeplinks-In-Correct-Browsing-Mode branch from b71eb23 to 7f47ae8 Compare July 17, 2024 22:04
@mobiletest-ci-bot
Copy link

Messages
📖 Project coverage: 31.89%
📖 Edited 18 files
📖 Created 0 files

Client.app: Coverage: 30.51

File Coverage
BrowserViewController.swift 5.42% ⚠️
BrowserCoordinator.swift 78.55%
MainMenuActionHelper.swift 39.81% ⚠️
RouteBuilder.swift 89.87%
WallpaperSettingsViewModel.swift 69.68%
JumpBackInViewModel.swift 38.34% ⚠️
TabManager.swift 70.73%
Route.swift 100.0%
TabManagerImplementation.swift 30.82% ⚠️
LegacyTabManager.swift 31.31% ⚠️

Generated by 🚫 Danger Swift against e38302f

Copy link
Contributor

mergify bot commented Jul 18, 2024

This pull request has conflicts when rebasing. Could you fix it @MattLichtenstein? 🙏

@adudenamedruby adudenamedruby requested review from adudenamedruby and removed request for razvanlitianu July 23, 2024 17:10
@adudenamedruby adudenamedruby added the Do Not Merge ⛔️ This issue is a work in progress and is not ready to land label Jul 30, 2024
@rvandermeulen rvandermeulen deleted the FXIOS-9407_Open-Deeplinks-In-Correct-Browsing-Mode branch August 16, 2024 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Do Not Merge ⛔️ This issue is a work in progress and is not ready to land
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants