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 extensions-common-manifest_url_handlers.cc.patch #14343

Closed
mariospr opened this issue Feb 24, 2021 · 2 comments · Fixed by brave/brave-core#8065
Closed

Remove extensions-common-manifest_url_handlers.cc.patch #14343

mariospr opened this issue Feb 24, 2021 · 2 comments · Fixed by brave/brave-core#8065

Comments

@mariospr
Copy link
Contributor

mariospr commented Feb 24, 2021

Description

This patch could be removed in favour of a chromium_src override to redefine ManifestURL::GetWebStoreURL() into a method that unconditionally returns an empty GURL

Desktop Brave version:

Brave 1.23.1 (Chromium 89.0.4389.58)

Miscellaneous Information:

This removal references a patch initially introduced with brave/brave-core#215

@mariospr mariospr added this to the 1.23.x - Nightly milestone Feb 24, 2021
@mariospr mariospr self-assigned this Feb 24, 2021
mariospr added a commit to brave/brave-core that referenced this issue Feb 24, 2021
This can be removed in favour of a chromium_src override to redefine
ManifestURL::GetWebStoreURL() into a method that unconditionally
returns an empty GURL.

Note that we need to also redefine GetHomepageURL() as it contains
a call to GetWebStoreURL(), otherwise upstream's version of the
GetWebStoreURL() function would be still be invoked via that way.
This is not ideal, but at least the code in GetHomepageURL() that
needs to be replicated in the chromium_src is very small and very
stable (hasn't changed in 8 years), so risk should be minimal.

Resolves brave/brave-browser#14343
@stephendonner
Copy link

stephendonner commented Mar 9, 2021

Verified PASSED using the testplan from brave/brave-core#8065

Brave 1.23.23 Chromium: 89.0.4389.72 (Official Build) nightly (x86_64)
Revision 3f345f156bfd157bd1bea06310e55f3fb2490359-refs/branch-heads/4389@{#1393}
OS macOS Version 11.2.3 (Build 20D91)

Verified that the PDF Viewer extension is bundled and not attributed to the Chrome Web Store, when launching with:

--args --enable-logging=stderr --vmodule="*/bat-native-ledger/*"=6,"*/brave_rewards/*"=6,"*/bat-native-ads/*"=6,"*/bat-native-confirmations/*"=6,"*/brave_ads/*"=6,"*/brave_user_model/*"=6 --brave-ads-staging --rewards=staging=true --show-component-extension-options

Screen Shot 2021-03-08 at 7 34 25 PM


Verification passed on


Brave | 1.23.52 Chromium: 89.0.4389.105 (Official Build) dev (64-bit)
-- | --
Revision | 14f44e21a9d539cd49c72468a29bfca4fa43f710-refs/branch-heads/4389_90@{#7}
OS | Windows 10 OS Version 2004 (Build 19041.867)


Verified that the PDF Viewer extension is bundled and not attributed to the Chrome Web Store, when launching with:
image


Verified passed with

Brave	1.23.63 Chromium: 89.0.4389.114 (Official Build) beta (64-bit)
Revision	1ea76e193b4fadb723bfea2a19a66c93a1bc0ca6-refs/branch-heads/4389@{#1616}
OS	Linux

Verified the test plan from brave/brave-core#8065 (comment) when launching with --args --enable-logging=stderr --show-component-extension-options:

Screen Shot 2021-04-06 at 9 42 43 AM

@srirambv
Copy link
Contributor

srirambv commented Apr 5, 2021

Removing OS/Android label as PDF is handled natively in Android and chrome://extensions doesn't resolve as extension support is not available on Android

@srirambv srirambv removed the OS/Android Fixes related to Android browser functionality label Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment