Fix Back button takes to r.duckduckgo.com #321
Conversation
| @@ -38,7 +38,7 @@ extension TabCollectionViewModel: NSSecureCoding { | |||
|
|
|||
| func encode(with coder: NSCoder) { | |||
| if let index = selectionIndex { | |||
| coder.encode(index) | |||
| coder.encode(index, forKey: NSSecureCodingKeys.selectionIndex) | |||
There was a problem hiding this comment.
additionally fix Selected Tab state restoration
|
Began looking through this on Friday, but I want to look through it some more tomorrow. |
| @@ -63,7 +63,7 @@ enum UserAgent { | |||
| regex("https://docs\\.google\\.com/spreadsheets/.*"): UserAgent.chrome, | |||
|
|
|||
| // use safari when serving up PDFs from duckduckgo directly | |||
| regex("https://duckduckgo\\.com/.*\\.pdf"): UserAgent.safari, | |||
| regex("https://duckduckgo\\.com/[^?]*\\.pdf"): UserAgent.safari, | |||
There was a problem hiding this comment.
fix Safari set to UserAgent for DDG pages for Search Queries ending with .pdf
There was a problem hiding this comment.
LGTM, tested this a bunch and compared it to Safari's behaviour with a custom user agent, our browser now behaves the same. This is a pretty clean approach in my opinion, a little bit of additional state is a pretty cheap price to pay for cleaning up some confusing and ugly navigation. 👍
Great job finding a minimal solution to this problem!
| if let self = self, | ||
| isUpgradable && navigationAction.isTargetingMainFrame, | ||
| let upgradedUrl = url.toHttps() { | ||
|
|
||
| if url == self.clientRedirectedDuringNavigationURL { | ||
| // Cancelled & Upgraded Client Redirect URL leaves wrong backForwardList record | ||
| // https://app.asana.com/0/inbox/1199237043628108/1201280322539473/1201353436736961 | ||
| self.webView.goBack() | ||
| self.webView.frozenCanGoBack = self.webView.canGoBack | ||
| self.webView.frozenCanGoForward = false | ||
| } | ||
|
|
||
| self.webView.load(upgradedUrl) | ||
| self.setConnectionUpgradedTo(upgradedUrl, navigationAction: navigationAction) | ||
| decisionHandler(.cancel) |
There was a problem hiding this comment.
Some day (not necessarily in this PR) I'd love to get some good testing going around our navigation logic in Tab.swift. Especially now that we have some additional state being modified depending on the value of clientRedirectedDuringNavigationURL. I'll have to think on this one some more - not sure it's a blocker for now.
We had this problem on iOS too, where our navigation logic became massive, and testing it was kind of a pain without some large refactors. Anyway, just thinking out loud 🙂
There was a problem hiding this comment.
The whole navigation decision function is long awaiting for being refactored, we're getting closer to it ;)
* develop: Avoid using a specific browser as the default. (#341) Closing animation duration changed to 0.15 (#333) Hiding the favicon view when favicon is nil (#326) export bookmarks (#339) Ensure ITP database is cleared (#328) Execute print user script in page content world (#238) move the next/previous handlers to the main view controller (#331) Search with DuckDuckGo in PDF Context Menu (#323) Fix Back button takes to r.duckduckgo.com (#321) Track the existing print handler to avoid DoS attacks. (#330) Minor copy fixes (#325) support cmd-option-left/right for navigating tabs (#324) Version 0.17.5 Debounce privacy entry point update (#309) Change major tracker network threshold (#319) Check for click handler on printing user script (#318) Fix Homepage navigation (#322)
* develop: (23 commits) Update the credit card model + UI tweaks (#342) 0.17.6 Add Usage Pixel (#336) Bump find-in-page to latest version (#337) Forward delete collision with suffix resolved (#334) Avoid using a specific browser as the default. (#341) Closing animation duration changed to 0.15 (#333) Hiding the favicon view when favicon is nil (#326) export bookmarks (#339) Ensure ITP database is cleared (#328) Execute print user script in page content world (#238) move the next/previous handlers to the main view controller (#331) Search with DuckDuckGo in PDF Context Menu (#323) Fix Back button takes to r.duckduckgo.com (#321) Track the existing print handler to avoid DoS attacks. (#330) Minor copy fixes (#325) support cmd-option-left/right for navigating tabs (#324) Version 0.17.5 Debounce privacy entry point update (#309) Change major tracker network threshold (#319) ...
Task/Issue URL: https://app.asana.com/0/inbox/1199237043628108/1201280322539473/1201353436736961/f
Tech Design URL: https://app.asana.com/0/inbox/1199237043628108/1201363901919881/1201353436736961/f
CC: @samsymons @tomasstrba
Description:
Fixes Redirect item landed in Back List
Steps to test this PR:
Testing checklist:
Internal references:
Software Engineering Expectations
Technical Design Template
When ready for review, remember to post the PR in MM