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

Remove patches/chrome-browser-profiles-profile_io_data.cc.patch #14238

Closed
mariospr opened this issue Feb 18, 2021 · 3 comments · Fixed by brave/brave-core#7995
Closed

Remove patches/chrome-browser-profiles-profile_io_data.cc.patch #14238

mariospr opened this issue Feb 18, 2021 · 3 comments · Fixed by brave/brave-core#7995

Comments

@mariospr
Copy link
Contributor

mariospr commented Feb 18, 2021

Description

The idea of this patch is simply to make sure that the brave:// scheme gets considered by ProfileIOData::IsHandledProtocol() along with other schemes, but we can achieve the same goal just using chromium_src overrides, so let's do that instead.

Desktop Brave version:

Brave 1.22.28 (Chromium 89.0.4389.48)

Miscellaneous Information:

For QA purposes: this cleanup touched code added to fix bugs #2861 and #2849, so please make sure behaviour remains the same.

There is a test plan in the associated PR in brave/brave-core#1196

@mariospr
Copy link
Contributor Author

Miscellaneous Information:

For QA purposes: this cleanup touched code added to fix bugs #2861 and #2849, so please make sure behaviour remains the same.

There is a test plan in the associated PR in brave/brave-core#1196

mariospr added a commit to brave/brave-core that referenced this issue Feb 19, 2021
The idea of this patch was simply to make sure that the brave://
scheme gets considered by ProfileIOData::IsHandledProtocol() along
with other schemes, but we can achieve the same goal just using
chromium_src overrides, so let's do that instead.

Fixes brave/brave-browser#14238
mariospr added a commit to brave/brave-core that referenced this issue Feb 22, 2021
The idea of this patch was simply to make sure that the brave://
scheme gets considered by ProfileIOData::IsHandledProtocol() along
with other schemes, but we can achieve the same goal just using
chromium_src overrides, so let's do that instead.

Fixes brave/brave-browser#14238
@stephendonner stephendonner added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Mar 4, 2021
@stephendonner
Copy link

Verified FIXED using the testplan from brave/brave-core#1196 with

Brave 1.22.51 Chromium: 89.0.4389.72 (Official Build) beta (x86_64)
Revision 3f345f156bfd157bd1bea06310e55f3fb2490359-refs/branch-heads/4389@{#1393}
OS macOS Version 11.2.2 (Build 20D80)

Test 1:
Navigate to https://world_languages.gitlab.io/bravepoc/
Click link
Check brave://settings is not loaded in new tab.

Confirmed that example.com opened in a new tab, instead

Screen Shot 2021-03-04 at 10 06 41 AM

Test 2:

Navigate to brave://welcome
Move to Change your search engine page in welcome page
Click Settings button
Check settings page is opened in new tab

The above testcase was for previous UI; I instead used the 3rd page in brave://welcome, which has Settings linked, as shown here.

Verified that it opened chrome://settings/privacy in a new tab.

Step 1 Step 2
Screen Shot 2021-03-04 at 10 12 17 AM Screen Shot 2021-03-04 at 10 19 04 AM

Test 3:

Open brave://settings
Right click on `brave://sync
Check Save Link As... is disabled

I confirmed via #brave-core in Slack that this is also outdated UI; @simonhong confirmed that the new test is actually to make sure that Save Link As... is enabled for a corresponding chrome:// link.

Verified that right-clicking and choosing Save Link As... for chrome://settings/braveSync resulted in the following dialog.

Step 1 Step 2
Screen Shot 2021-03-04 at 10 18 19 AM Screen Shot 2021-03-04 at 10 17 16 AM

@stephendonner stephendonner added QA Pass-macOS and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Mar 4, 2021
@srirambv
Copy link
Contributor

srirambv commented Mar 9, 2021

Verification passed on OnePlus 6T with Android 10 running 1.22.54 x64 beta build


Verification passed on Samsung Tab A with Android 10 running 1.22.54 x64 beta build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants