Skip to content

[iOS] - Fix Private Browsing Not working on Open Window #28621

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Apr 17, 2025
Merged

[iOS] - Fix Private Browsing Not working on Open Window #28621

merged 3 commits into from
Apr 17, 2025

Conversation

Brandon-T
Copy link
Contributor

Info

  • The isPrivateBrowsing: Bool = false causes bugs where the parameter is defaulted to false, and usage of the openInNewTab can accidentally not notice that it's false by default. This can cause incorrect usage of the function.

Summary

  • Remove isPrivateBrowsing: Bool = false for private browsing (removed default parameter).
  • Update all uses to explicitly specify private browsing.

Resolves brave/brave-browser#45389

@Brandon-T Brandon-T added CI/skip-android Do not run CI builds for Android CI/skip-macos-x64 Do not run CI builds for macOS x64 CI/skip-windows-x64 Do not run CI builds for Windows x64 unused-CI/skip-linux-x64 Do not run CI builds for Linux x64 CI/skip-macos-arm64 Do not run CI builds for macOS arm64 CI/skip-teamcity Do not run builds in TeamCity labels Apr 11, 2025
@Brandon-T Brandon-T self-assigned this Apr 11, 2025
@Brandon-T Brandon-T requested a review from a team as a code owner April 11, 2025 22:18
@Brandon-T Brandon-T changed the title [iOS] - Fix Private Browsing Not working on Long-Press New Tab [iOS] - Fix Private Browsing Not working on Open Window Apr 11, 2025
Brandon-T
Brandon-T commented Apr 11, 2025
View reviewed changes
ios/brave-ios/App/iOS/Delegates/SceneDelegate.swift Outdated
BrowserState.setWindowInfo(
for: scene.session,
windowId: windowId.uuidString,
isPrivate: isPrivate
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: These lines have nothing to do with it and don't cause issues. They were updated only because I noticed them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are these changed if they have nothing to do with the other changes? You also now pass in isPrivate instead of false, isn't that a change in behaviour?

Copy link
Contributor Author

@Brandon-T Brandon-T Apr 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I changed them just in case. Technically they should be isPrivate and not false so I changed them. However, the change doesn't affect the PR. IE: It doesn't affect the multi-window opening as per the ticket. In fact, I don't see anything affected with or without the change.

I can remove it if we want to minimize any potential impacts, since it's not related to the bug.

EDIT: Reverted.

@Brandon-T Brandon-T requested a review from kylehickinson April 14, 2025 19:48
@Brandon-T Brandon-T force-pushed the bugfix/PrivateBrowsingFlags branch from e69ff53 to 248d314 Compare April 15, 2025 17:12
kylehickinson
kylehickinson reviewed Apr 15, 2025
View reviewed changes
ios/brave-ios/App/iOS/Delegates/SceneDelegate.swift Outdated
scene.browserViewController?.switchToTabForURLOrOpen(
url,
isPrivate: isPrivateBrowsing,
isPrivileged: true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Realize it was privileged before but is there a reason this one is opened with privilege?

Copy link
Contributor Author

@Brandon-T Brandon-T Apr 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

:o good catch. No it should NOT be privileged. It's a custom user activity for shortcuts as per the PR it was added in: https://github.com/brave/brave-core/pull/22970/files

EDIT: Fixed.

@Brandon-T Brandon-T force-pushed the bugfix/PrivateBrowsingFlags branch from 6682d31 to aaebde1 Compare April 16, 2025 14:30
@github-actions GitHub Actions
Copy link
Contributor

Chromium major version is behind target branch (135.0.7049.100 vs 136.0.7103.25). Please rebase.

@github-actions github-actions bot added the chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch label Apr 16, 2025
@Brandon-T Brandon-T force-pushed the bugfix/PrivateBrowsingFlags branch from aaebde1 to 18102fe Compare April 17, 2025 14:09
@github-actions github-actions bot removed the chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch label Apr 17, 2025
@Brandon-T Brandon-T merged commit 442e6cc into master Apr 17, 2025
18 checks passed
@Brandon-T Brandon-T deleted the bugfix/PrivateBrowsingFlags branch April 17, 2025 16:09
@github-actions github-actions bot added this to the 1.79.x - Nightly milestone Apr 17, 2025
brave-builds added a commit that referenced this pull request Apr 17, 2025
@brave-builds
Uplift of #28621 (squashed) to beta
818a8bc
brave-builds added a commit that referenced this pull request Apr 17, 2025
@brave-builds
Uplift of #28621 (squashed) to release
9474931
This was referenced Apr 17, 2025
Merged
Closed
@brave-builds
Copy link
Collaborator

Released in v1.79.75

@Uni-verse
Copy link
Contributor

Verified on iPad (9th Gen) running iOS 18.3.1 using version 1.79.77

  • Reproduced the original issue on an earlier build, then installed 1.79.77, enabled Private Browsing Only and used test steps in Open in new private window results in normal window brave-browser#45389 (comment) to ensure the following:
    • Link are opened in a new private window when using "Open in New Private Window" from the context menu.
    • Windows are successfully opening in Private mode in both portrait and landscape orientation.
    • Private tabs can be opened/loaded in the background and also from the toolbar.
example example example
IMG_0507 2 IMG_0508 3 IMG_0509 3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kylehickinson kylehickinson kylehickinson approved these changes

@stoletheminerals stoletheminerals stoletheminerals approved these changes

Assignees

@Brandon-T Brandon-T

Labels
CI/skip-android Do not run CI builds for Android CI/skip-macos-arm64 Do not run CI builds for macOS arm64 CI/skip-macos-x64 Do not run CI builds for macOS x64 CI/skip-teamcity Do not run builds in TeamCity CI/skip-windows-x64 Do not run CI builds for Windows x64 security unused-CI/skip-linux-x64 Do not run CI builds for Linux x64
Projects
None yet
Milestone
1.79.x - Beta
Development

Successfully merging this pull request may close these issues.

Open in new private window results in normal window
5 participants
@Brandon-T @brave-builds @Uni-verse @kylehickinson @stoletheminerals