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 chrome-common-chrome_content_client_constants.cc.patch #14338

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

Remove chrome-common-chrome_content_client_constants.cc.patch #14338

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

Comments

@mariospr
Copy link
Contributor

Description

This patch could be removed in favour of chromium_src overrides to redefine the values for the constants we're interested in.

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#2342

@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 patch can be removed in favour of chromium_src overrides to
redefine the values for the constants we're interested in.

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

stephendonner commented Mar 5, 2021

Verified PASSED with the testplan from brave/brave-core#2342, using nightly build

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

Test fix for brave/brave-browser#3694

  1. Go to https://docs.google.com and login
  2. Create a new Google Sheets document
  3. Type some values in there
  4. Pick File => Print from the Google apps menu (not the browser menu)
  5. Try to print document
  6. Document should actually print (you should not be prompted to save as PDF)

NOTE: Same behavior on 1.21.73 Chromium: 89.0.4389.72 (Official Build) (x86_64); this looks to be #11913

Screen Shot 2021-03-04 at 4 43 34 PM

Test fix for brave/brave-browser#884

  1. Go to http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.144.7135&rep=rep1&type=pdf
  2. Document should load properly

Screen Shot 2021-03-04 at 4 45 42 PM

Test fix for brave/brave-browser#4424

  1. Go to http://pdfkit.org/demo/browser.html
  2. Document on right hand side should load properly

Screen Shot 2021-03-04 at 4 46 00 PM

Embed test (possible fix for brave/brave-browser#3119)

  1. Go to https://pdfobject.com/examples/detection.html
  2. Message This browser supports inline PDFs should be shown (green color)

Screen Shot 2021-03-04 at 4 45 33 PM

Test that PDF.js is still installable

  1. Go to https://chrome.google.com/webstore/detail/pdf-viewer/oemmndcbldboiebfnladdacbdfmadadm
  2. Verify that you can install PDF.js
  3. Verify that PDF.js puts a browser action button in the extension area
  4. Verify that PDF.js shows in brave://extensions
  5. Open a PDF document and verify PDF.js is used

Screen Shot 2021-03-04 at 4 47 07 PM

Screen Shot 2021-03-04 at 4 48 12 PM

Screen Shot 2021-03-04 at 4 48 41 PM

Test that PDF.js does not show

  1. Have an existing profile BEFORE testing this. Make sure PDF.js works
  2. Upgrade to a build with this fix
  3. Launch Brave from the command line with --show-component-extension-options
  4. Verify PDF.js does NOT show in brave://extensions

Screen Shot 2021-03-04 at 5 22 39 PM

  1. Verify PDF.js does NOT show in brave://components

Screen Shot 2021-03-04 at 5 22 19 PM


Verification passed on

Brave | 1.23.59 Chromium: 89.0.4389.114 (Official Build) dev (64-bit)
-- | --
Revision | 1ea76e193b4fadb723bfea2a19a66c93a1bc0ca6-refs/branch-heads/4389@{#1616}
OS | Windows 10 OS Version 2004 (Build 19041.867)

Test fix for brave/brave-browser#3694

  1. Go to https://docs.google.com and login
  2. Create a new Google Sheets document
  3. Type some values in there
  4. Pick File => Print from the Google apps menu (not the browser menu)
  5. Try to print document
  6. Document should actually print (you should not be prompted to save as PDF)

Click on Print icon in the google apps menu for excel doc is still prompting save the file instead of Print dialogue, the issue is not fixed, encountered the issue #11913

image

Test fix for brave/brave-browser#884

  1. Go to http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.144.7135&rep=rep1&type=pdf
  2. Document should load properly
    image

Test fix for brave/brave-browser#4424

  1. Go to http://pdfkit.org/demo/browser.html
  2. Document on right hand side should load properly
    image

Embed test (possible fix for brave/brave-browser#3119)

  1. Go to https://pdfobject.com/examples/detection.html
  2. Message This browser supports inline PDFs should be shown (green color)
    image

Test that PDF.js is still installable

  1. Go to https://chrome.google.com/webstore/detail/pdf-viewer/oemmndcbldboiebfnladdacbdfmadadm
  2. Verify that you can install PDF.js
  3. Verify that PDF.js puts a browser action button in the extension area
  4. Verify that PDF.js shows in brave://extensions
  5. Open a PDF document and verify PDF.js is used

image
image
image
image

Test that PDF.js does not show

  1. Have an existing profile BEFORE testing this. Make sure PDF.js works
  2. Upgrade to a build with this fix
  3. Launch Brave from the command line with --show-component-extension-options
  4. Verify PDF.js/ PDF viewer show in brave://extensions

Installed 1.22.52 and installed PDF.js extension https://chrome.google.com/webstore/detail/pdf-viewer/oemmndcbldboiebfnladdacbdfmadadm/related

Upgraded the profile to 1.23.59 and launched the browser using brave.exe --show-component-extension-options and ensured PDF.js / PDF viewer shown in brave://extensions
image

Verify PDF.js does NOT show in brave://components
image


Verification PASSED on Ubuntu 20.04 x64 using the following build:

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

Test fix for brave/brave-browser#3694

  1. Go to https://docs.google.com and login
  2. Create a new Google Sheets document
  3. Type some values in there
  4. Pick File => Print from the Google apps menu (not the browser menu)
  5. Try to print document
  6. Document should actually print (you should not be prompted to save as PDF)

NOTE: Ran into #11913 when going through this case. Asking me to save the PDF rather than printing it which is known.

Example Example
Screen Shot 2021-04-07 at 3 05 05 PM Screen Shot 2021-04-07 at 3 05 16 PM

Test fix for brave/brave-browser#884

  1. Go to http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.144.7135&rep=rep1&type=pdf
  2. Document should load properly

Screen Shot 2021-04-07 at 3 07 42 PM

Test fix for brave/brave-browser#4424

  1. Go to http://pdfkit.org/demo/browser.html
  2. Document on right hand side should load properly

Screen Shot 2021-04-07 at 3 09 19 PM

Embed test (possible fix for brave/brave-browser#3119)

  1. Go to https://pdfobject.com/examples/detection.html
  2. Message This browser supports inline PDFs should be shown (green color)

Screen Shot 2021-04-07 at 3 19 17 PM

Test that PDF.js is still installable

  1. Go to https://chrome.google.com/webstore/detail/pdf-viewer/oemmndcbldboiebfnladdacbdfmadadm
  2. Verify that you can install PDF.js
  3. Verify that PDF.js puts a browser action button in the extension area
  4. Verify that PDF.js shows in brave://extensions
  5. Open a PDF document and verify PDF.js is used

Screen Shot 2021-04-07 at 3 21 08 PM

Screen Shot 2021-04-07 at 3 21 59 PM

Screen Shot 2021-04-07 at 3 22 30 PM

Test that PDF.js does not show

  1. Have an existing profile BEFORE testing this. Make sure PDF.js works
  2. Upgrade to a build with this fix
  3. Launch Brave from the command line with --show-component-extension-options
  4. Verify PDF.js does NOT show in brave://extensions

image

Note: Found #15185 while running through the above case. Looks like PDF Viewer isn't being removed from brave://extensions when launching Brave via --show-component-extension-options. @stephendonner reproduced as well.

  1. Verify PDF.js does NOT show in brave://components

image (1)

@stephendonner stephendonner added QA Pass-macOS and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Mar 5, 2021
@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

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