Issue metadata
Sign in to add a comment
|
Since Chrome 68.x, PDF downloads fail because Chrome think the Content Security Policy is blocking the download.
Reported by
patrickf...@gmail.com,
Aug 29
|
||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36 Example URL: https://smart911.rave411.com/rcv/login Steps to reproduce the problem: Please enable the chrome option "Download PDF files instead of automatically opening them in Chrome" under PDF documents. 1)When in click on a ticket from the ticket list on the left side of the page. 2)This will open a smartlet in the middle of the page. (Header of smartlet is Smart911 Profile. 3)On the addresses section of the smartlet, click on first address (74 Winslow ave) which will display meta data for that address. 4) Click the "view uploads document" link next to the label "Building Plans", this will download a pdf file. 5)Click the large "CLOSE DOCUMENT" button. What is the expected behavior? PDF downloads to the /downloads folder. What went wrong? Erroneously the browser (since 68.x) blocks the PDF download due to what it thinks is a breach of the Content Security Policy. Did this work before? Yes Works in all Chrome versions 67.x and back. Stopped working in 68.x. Chrome version: 68.0.3440.106 Channel: stable OS Version: 10.0 Flash Version: Here is the Content Security Policy. Content-Security-Policy: default-src 'self'; font-src 'self' data: *.gstatic.com; frame-src 'self' data: blob: www.nbcnews.com www.youtube.com; connect-src 'self' *.mapbox.com wss://smart911.rave411.com translate.googleapis.com; child-src 'self' *.youtube.com wss://smart911.rave411.com; object-src 'self' 'unsafe-inline' blob: data: smart911.rave411.com; style-src 'self' 'unsafe-inline' *.googleapis.com; img-src * blob: data: *.mapbox.com *.rave411.com; script-src 'self' 'unsafe-inline' 'unsafe-eval' *.google.com *.google-analytics.com *.googleapis.com *.mapbox.com www.sc.pages05.net
,
Aug 30
Tried testing the issue on reported chrome version #68.0.3440.106 using windows 10, by following the below steps. Steps: ===== 1.Launched chrome. 2.Navigated to chrome://settings. 3.Enabled the chrome option "Download PDF files instead of automatically opening them in Chrome" under PDF documents. 4.Navigated to 'https://smart911.rave411.com/rcv/login', but the site did not open and error 'The site cannot be reached' is displayed. Attached screencast for referece. @reporter : Could you please check the attched screencast and let us know if anything is being missed here. Request you to provide a sample URL/file that reproduces the issue so that it would be really helpful for triaging the issue. Thanks.!
,
Aug 30
swarnasree.mukkala@chromium.org, thanks for having a look. The site is definitely up and the important thing when reproducing this is to have the Dev Tools JS Console open where you will see the exception as soon as you click the link to open the PDF. All I can ask is to try again as the link is correct and site is up and running. https://smart911.rave411.com/rcv/login
,
Aug 30
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 31
Tried testing the issue on reported chrome version #68.0.3440.106, by using Windows 10 following steps as per comment#0. Observed that the site cannot be reached. Attached screencast for reference. @reporter: Could you please provide any other sample URL/File that reproduces the issue, so that it would be really helpful for further triaging. Thanks.!
,
Aug 31
swarnasree.mukkala@chromium.org, I'm at a loss. The site is up and running, I can get to the site from my AWS instances, from my work site and home. This seems like a network issue from your side. The error from the browser seems like it never even made it to the site. Can you try some of our other sites? https://www.smart911.com https://smart911.rave411.com https://www.rave411.com https://www.getrave.com Let me know if any or all work? Thank you, Patrick
,
Aug 31
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 31
,
Sep 3
As per comment #6, tried opening the given links observed that the first two links gets the error as "The site cannot be reached" and second two links opens. Below are the observations when these latter two links opens. Observations: =========== 1.They navigate to a page where a page loads with a text box to enter a site value. 2.Entered the site value as "smart911" . 3.They navigate to the page where it asks for a login with username and password, tried searching how to register but could not find it out. Note: It happens for all sites which appear in suggestions while entering "smart911" in site value Attached screencast for reference. @reporter : Could you please provide sample credentials to login into these sites. Thanks.!
,
Sep 3
Microsoft Windows [Version 10.0.17134.228] (c) 2018 Microsoft Corporation. All rights reserved. C:\Users\Pat>nslookup smart911.rave411.com Server: google-public-dns-a.google.com Address: 8.8.8.8 Non-authoritative answer: Name: smart911.rave411.com Address: 205.237.106.30 Can you do an NSLookup on the 2 site that you cannot connect to and compare results to mine shown below? C:\Users\Pat>nslookup www.smart911.com Server: google-public-dns-a.google.com Address: 8.8.8.8 Non-authoritative answer: Name: www.smart911.com Address: 75.98.95.168
,
Sep 3
Can you do an NSLookup on the 2 sites that you cannot connect to and compare results to mine shown below? Microsoft Windows [Version 10.0.17134.228] (c) 2018 Microsoft Corporation. All rights reserved. C:\Users\Pat>nslookup smart911.rave411.com Server: google-public-dns-a.google.com Address: 8.8.8.8 Non-authoritative answer: Name: smart911.rave411.com Address: 205.237.106.30 C:\Users\Pat>nslookup www.smart911.com Server: google-public-dns-a.google.com Address: 8.8.8.8 Non-authoritative answer: Name: www.smart911.com Address: 75.98.95.168
,
Sep 3
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 5
@reporter: Before proceeding to further investigation of the issue, could you please confirm if the issue is seen on recent chrome version #69.0.3947.81, as recently the stable has been rolled out to M-69.You can download the latest chrome builds from the link "https://www.chromium.org/getting-involved/dev-channel". Thanks.!
,
Sep 5
Same issue in 70.0.3534.4 (Official Build) dev (64-bit) (cohort: Dev)
,
Sep 5
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 6
The issue seems to similar to issue: 879149 . Requesting dev team to check the issue and confirm whether both issues are similar or not and assigning this issue to tommycli for further inputs on the issue. As per comment#2, comment#5 and comment#9 unable to test the issue at TE-end, hence removing Needs-Bisect label. Thanks.!
,
Sep 6
The provided login link doesn't work. See screenshot.
That being said, if you are trying to force a PDF download, I suggest the following techniques:
I suggest one of four ways to accomplish that (ordered from most preferred to least preferred)
1. Use the HTTP header Content-Disposition: attachment on the server side:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition
2. Or use Content-Type: application/octet-stream on the server
3. Or use the HTML5 download attribute: References:
https://stackoverflow.com/questions/3077242/force-download-a-pdf-link-using-javascript-ajax-jquery
https://www.w3schools.com/Tags/att_a_download.asp
4. There's another pure-Javascript approach listed here:
https://stackoverflow.com/questions/3077242/force-download-a-pdf-link-using-javascript-ajax-jquery/29266135#29266135
With any of the above recommendations users should no longer need to Enable chrome://settings/content/pdfDocuments for it to work.
,
Sep 6
Please try again, you just happen to try and connect when server instance was down. Sorry about that. This Content Security Policy we currently have is completely proper and only fails with Chrome starting with version 68.x. All other browsers work, IE, Edge, Firefox and Opera.
,
Sep 6
What should the login credentials be? I think likely the Content Security Policy warning is a red herring. Are you trying to use an IFRAME to trigger a PDF download?
,
Sep 6
Creds: chrome / Chromepass!
,
Sep 6
The Content Security Policy error is causing the download to fail. That same console error (not a warning) did not show in chrome 67.x and earlier. The error says it is "refusing" to load data.
,
Oct 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ad61fb278ef03510d0e5e2070018a05adc34489c commit ad61fb278ef03510d0e5e2070018a05adc34489c Author: Tommy C. Li <tommycli@chromium.org> Date: Fri Oct 05 17:53:23 2018 Click to Open PDF: Put placeholder HTML in IFRAMEs directly Previously, when the user has a disabled PDF plugin (or no PDF plugin) and the website has an IFRAMEed PDF, when Click to Open PDF was enabled, we would inject a fake <object> tag into the IFRAME to force a PDF plugin placeholder to appear. This approach is problematic with CSP, as CSP may forbid OBJECT tags from loading in IFRAMEs. This CL instead injects the placeholder HTML directly into the IFRAME. In the IFRAME case, the button is now a plain link, which should allow the user to click it and download the PDF even if CSP forbids <object> tags or JavaScript. It still may trigger a CSP warning if IFRAME is prohibited from running JavaScript, but that only hurts keyboard-accessibility, and doesn't prevent the mainstream use case from working. This CL also prevents the <object> tag from auto-opening the PDF after download, which was probably a mistake, since that overrides the user configurable "Always open with system viewer" option. Bug: 887752, 879149 , 878871 Change-Id: I900c08331d2cfc96ea7cd1cd46ea594445b0920b Reviewed-on: https://chromium-review.googlesource.com/c/1246956 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#597192} [modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/browser/download/chrome_download_manager_delegate.cc [modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/browser/pdf/pdf_extension_test.cc [modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/browser/plugins/pdf_iframe_navigation_throttle.cc [modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/browser/plugins/pdf_plugin_placeholder_observer.cc [modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/common/BUILD.gn [modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/common/DEPS [delete] https://crrev.com/d92a8619b6a7c29294823c7ccefcbb1d7b43912c/chrome/common/pdf_uma.cc [add] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/common/pdf_util.cc [rename] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/common/pdf_util.h [modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/renderer/chrome_content_renderer_client.cc [modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/renderer/plugins/pdf_plugin_placeholder.cc [modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/renderer/resources/plugins/pdf_plugin.html [add] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/test/data/pdf/pdf_embed_csp.html [add] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/test/data/pdf/pdf_iframe_csp.html
,
Oct 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2a68a0f0255d80a83b49e2709f30818418960f46 commit 2a68a0f0255d80a83b49e2709f30818418960f46 Author: Marijn Kruisselbrink <mek@chromium.org> Date: Fri Oct 05 21:43:19 2018 Revert "Click to Open PDF: Put placeholder HTML in IFRAMEs directly" This reverts commit ad61fb278ef03510d0e5e2070018a05adc34489c. Reason for revert: New PDFPluginDisabledTest.IframePdfPlaceholderWithCSP test is failing in https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.13%20Tests%20%28dbg%29/5617 Original change's description: > Click to Open PDF: Put placeholder HTML in IFRAMEs directly > > Previously, when the user has a disabled PDF plugin (or no PDF plugin) > and the website has an IFRAMEed PDF, when Click to Open PDF was enabled, > we would inject a fake <object> tag into the IFRAME to force a PDF > plugin placeholder to appear. > > This approach is problematic with CSP, as CSP may forbid OBJECT tags > from loading in IFRAMEs. > > This CL instead injects the placeholder HTML directly into the IFRAME. > > In the IFRAME case, the button is now a plain link, which should allow > the user to click it and download the PDF even if CSP forbids <object> > tags or JavaScript. > > It still may trigger a CSP warning if IFRAME is prohibited from running > JavaScript, but that only hurts keyboard-accessibility, and doesn't > prevent the mainstream use case from working. > > This CL also prevents the <object> tag from auto-opening the PDF after > download, which was probably a mistake, since that overrides the user > configurable "Always open with system viewer" option. > > Bug: 887752, 879149 , 878871 > Change-Id: I900c08331d2cfc96ea7cd1cd46ea594445b0920b > Reviewed-on: https://chromium-review.googlesource.com/c/1246956 > Reviewed-by: Jochen Eisinger <jochen@chromium.org> > Reviewed-by: Lei Zhang <thestig@chromium.org> > Commit-Queue: Tommy Li <tommycli@chromium.org> > Cr-Commit-Position: refs/heads/master@{#597192} TBR=thestig@chromium.org,tommycli@chromium.org,jochen@chromium.org,mkwst@chromium.org Change-Id: I8a5554c0489ec76a4adb35ca93afef9c4354316e No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 887752, 879149 , 878871 Reviewed-on: https://chromium-review.googlesource.com/c/1265826 Reviewed-by: Marijn Kruisselbrink <mek@chromium.org> Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> Cr-Commit-Position: refs/heads/master@{#597315} [modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/browser/download/chrome_download_manager_delegate.cc [modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/browser/pdf/pdf_extension_test.cc [modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/browser/plugins/pdf_iframe_navigation_throttle.cc [modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/browser/plugins/pdf_plugin_placeholder_observer.cc [modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/common/BUILD.gn [modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/common/DEPS [add] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/common/pdf_uma.cc [rename] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/common/pdf_uma.h [delete] https://crrev.com/7dad1c4475b73eacba0029db21683bcda832f8c7/chrome/common/pdf_util.cc [modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/renderer/chrome_content_renderer_client.cc [modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/renderer/plugins/pdf_plugin_placeholder.cc [modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/renderer/resources/plugins/pdf_plugin.html [delete] https://crrev.com/7dad1c4475b73eacba0029db21683bcda832f8c7/chrome/test/data/pdf/pdf_embed_csp.html [delete] https://crrev.com/7dad1c4475b73eacba0029db21683bcda832f8c7/chrome/test/data/pdf/pdf_iframe_csp.html
,
Oct 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/49eacb581082e7a2469b613ad329e5dd75904616 commit 49eacb581082e7a2469b613ad329e5dd75904616 Author: Tommy C. Li <tommycli@chromium.org> Date: Tue Oct 09 21:19:26 2018 Reland "Click to Open PDF: Put placeholder HTML in IFRAMEs directly" This is a reland of ad61fb278ef03510d0e5e2070018a05adc34489c Original change's description: > Click to Open PDF: Put placeholder HTML in IFRAMEs directly > > Previously, when the user has a disabled PDF plugin (or no PDF plugin) > and the website has an IFRAMEed PDF, when Click to Open PDF was enabled, > we would inject a fake <object> tag into the IFRAME to force a PDF > plugin placeholder to appear. > > This approach is problematic with CSP, as CSP may forbid OBJECT tags > from loading in IFRAMEs. > > This CL instead injects the placeholder HTML directly into the IFRAME. > > In the IFRAME case, the button is now a plain link, which should allow > the user to click it and download the PDF even if CSP forbids <object> > tags or JavaScript. > > It still may trigger a CSP warning if IFRAME is prohibited from running > JavaScript, but that only hurts keyboard-accessibility, and doesn't > prevent the mainstream use case from working. > > This CL also prevents the <object> tag from auto-opening the PDF after > download, which was probably a mistake, since that overrides the user > configurable "Always open with system viewer" option. > > Bug: 887752, 879149 , 878871 > Change-Id: I900c08331d2cfc96ea7cd1cd46ea594445b0920b > Reviewed-on: https://chromium-review.googlesource.com/c/1246956 > Reviewed-by: Jochen Eisinger <jochen@chromium.org> > Reviewed-by: Lei Zhang <thestig@chromium.org> > Commit-Queue: Tommy Li <tommycli@chromium.org> > Cr-Commit-Position: refs/heads/master@{#597192} Bug: 887752, 879149 , 878871 Change-Id: I2778524eac784186b141d7caee3801af3863fd2e Reviewed-on: https://chromium-review.googlesource.com/c/1269442 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/master@{#598083} [modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/browser/download/chrome_download_manager_delegate.cc [modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/browser/pdf/pdf_extension_test.cc [modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/browser/plugins/pdf_iframe_navigation_throttle.cc [modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/browser/plugins/pdf_plugin_placeholder_observer.cc [modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/common/BUILD.gn [modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/common/DEPS [delete] https://crrev.com/47e1665f4f428d7c42ef2373f8e6a9ad8848a990/chrome/common/pdf_uma.cc [add] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/common/pdf_util.cc [rename] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/common/pdf_util.h [modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/renderer/chrome_content_renderer_client.cc [modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/renderer/plugins/pdf_plugin_placeholder.cc [modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/renderer/resources/plugins/pdf_plugin.html [add] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/test/data/pdf/pdf_embed_csp.html [add] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/test/data/pdf/pdf_iframe_csp.html
,
Oct 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f commit d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f Author: Findit <findit-for-me@appspot.gserviceaccount.com> Date: Wed Oct 10 06:26:18 2018 Revert "Reland "Click to Open PDF: Put placeholder HTML in IFRAMEs directly"" This reverts commit 49eacb581082e7a2469b613ad329e5dd75904616. Reason for revert: Findit (https://goo.gl/kROfz5) identified CL at revision 598083 as the culprit for flakes in the build cycles as shown on: https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vNDllYWNiNTgxMDgyZTdhMjQ2OWI2MTNhZDMyOWU1ZGQ3NTkwNDYxNgw Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/linux-xenial-rel/4027 Sample Failed Step: network_service_browser_tests on Ubuntu-16.04 Sample Flaky Test: PDFPluginDisabledTest.EmbedPdfPlaceholderWithCSP Original change's description: > Reland "Click to Open PDF: Put placeholder HTML in IFRAMEs directly" > > This is a reland of ad61fb278ef03510d0e5e2070018a05adc34489c > > Original change's description: > > Click to Open PDF: Put placeholder HTML in IFRAMEs directly > > > > Previously, when the user has a disabled PDF plugin (or no PDF plugin) > > and the website has an IFRAMEed PDF, when Click to Open PDF was enabled, > > we would inject a fake <object> tag into the IFRAME to force a PDF > > plugin placeholder to appear. > > > > This approach is problematic with CSP, as CSP may forbid OBJECT tags > > from loading in IFRAMEs. > > > > This CL instead injects the placeholder HTML directly into the IFRAME. > > > > In the IFRAME case, the button is now a plain link, which should allow > > the user to click it and download the PDF even if CSP forbids <object> > > tags or JavaScript. > > > > It still may trigger a CSP warning if IFRAME is prohibited from running > > JavaScript, but that only hurts keyboard-accessibility, and doesn't > > prevent the mainstream use case from working. > > > > This CL also prevents the <object> tag from auto-opening the PDF after > > download, which was probably a mistake, since that overrides the user > > configurable "Always open with system viewer" option. > > > > Bug: 887752, 879149 , 878871 > > Change-Id: I900c08331d2cfc96ea7cd1cd46ea594445b0920b > > Reviewed-on: https://chromium-review.googlesource.com/c/1246956 > > Reviewed-by: Jochen Eisinger <jochen@chromium.org> > > Reviewed-by: Lei Zhang <thestig@chromium.org> > > Commit-Queue: Tommy Li <tommycli@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#597192} > > Bug: 887752, 879149 , 878871 > Change-Id: I2778524eac784186b141d7caee3801af3863fd2e > Reviewed-on: https://chromium-review.googlesource.com/c/1269442 > Reviewed-by: Jochen Eisinger <jochen@chromium.org> > Commit-Queue: Tommy Li <tommycli@chromium.org> > Commit-Queue: Lei Zhang <thestig@chromium.org> > Cr-Commit-Position: refs/heads/master@{#598083} Change-Id: Ic7554c10dea8131f70f9ff2d6dbb7202e49a142f No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 887752, 879149 , 878871, 892818 Reviewed-on: https://chromium-review.googlesource.com/c/1271929 Cr-Commit-Position: refs/heads/master@{#598224} [modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/browser/download/chrome_download_manager_delegate.cc [modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/browser/pdf/pdf_extension_test.cc [modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/browser/plugins/pdf_iframe_navigation_throttle.cc [modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/browser/plugins/pdf_plugin_placeholder_observer.cc [modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/common/BUILD.gn [modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/common/DEPS [add] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/common/pdf_uma.cc [rename] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/common/pdf_uma.h [delete] https://crrev.com/c27ea6d66efa8d360b86092d746f804ee81b7c8f/chrome/common/pdf_util.cc [modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/renderer/chrome_content_renderer_client.cc [modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/renderer/plugins/pdf_plugin_placeholder.cc [modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/renderer/resources/plugins/pdf_plugin.html [delete] https://crrev.com/c27ea6d66efa8d360b86092d746f804ee81b7c8f/chrome/test/data/pdf/pdf_embed_csp.html [delete] https://crrev.com/c27ea6d66efa8d360b86092d746f804ee81b7c8f/chrome/test/data/pdf/pdf_iframe_csp.html
,
Oct 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a commit ce661f8f18e8d2a20a0e8dbae259ad8912aa166a Author: Tommy C. Li <tommycli@chromium.org> Date: Wed Oct 10 23:33:02 2018 Reland "Reland "Click to Open PDF: Put placeholder HTML in IFRAMEs directly"" This is a reland of 49eacb581082e7a2469b613ad329e5dd75904616 Original change's description: > Reland "Click to Open PDF: Put placeholder HTML in IFRAMEs directly" > > This is a reland of ad61fb278ef03510d0e5e2070018a05adc34489c > > Original change's description: > > Click to Open PDF: Put placeholder HTML in IFRAMEs directly > > > > Previously, when the user has a disabled PDF plugin (or no PDF plugin) > > and the website has an IFRAMEed PDF, when Click to Open PDF was enabled, > > we would inject a fake <object> tag into the IFRAME to force a PDF > > plugin placeholder to appear. > > > > This approach is problematic with CSP, as CSP may forbid OBJECT tags > > from loading in IFRAMEs. > > > > This CL instead injects the placeholder HTML directly into the IFRAME. > > > > In the IFRAME case, the button is now a plain link, which should allow > > the user to click it and download the PDF even if CSP forbids <object> > > tags or JavaScript. > > > > It still may trigger a CSP warning if IFRAME is prohibited from running > > JavaScript, but that only hurts keyboard-accessibility, and doesn't > > prevent the mainstream use case from working. > > > > This CL also prevents the <object> tag from auto-opening the PDF after > > download, which was probably a mistake, since that overrides the user > > configurable "Always open with system viewer" option. > > > > Bug: 887752, 879149 , 878871 > > Change-Id: I900c08331d2cfc96ea7cd1cd46ea594445b0920b > > Reviewed-on: https://chromium-review.googlesource.com/c/1246956 > > Reviewed-by: Jochen Eisinger <jochen@chromium.org> > > Reviewed-by: Lei Zhang <thestig@chromium.org> > > Commit-Queue: Tommy Li <tommycli@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#597192} > > Bug: 887752, 879149 , 878871 > Change-Id: I2778524eac784186b141d7caee3801af3863fd2e > Reviewed-on: https://chromium-review.googlesource.com/c/1269442 > Reviewed-by: Jochen Eisinger <jochen@chromium.org> > Commit-Queue: Tommy Li <tommycli@chromium.org> > Commit-Queue: Lei Zhang <thestig@chromium.org> > Cr-Commit-Position: refs/heads/master@{#598083} TBR=jochen@chromium.org Bug: 887752, 879149 , 878871 Change-Id: I6e6053e32a5d8acbeb18a32ba067a885ce915fe2 Reviewed-on: https://chromium-review.googlesource.com/c/1273985 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/master@{#598570} [modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/browser/download/chrome_download_manager_delegate.cc [modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/browser/plugins/pdf_iframe_navigation_throttle.cc [modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/browser/plugins/pdf_plugin_placeholder_observer.cc [modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/common/BUILD.gn [modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/common/DEPS [delete] https://crrev.com/5ea805f821e4922f5ae39e2ed214ef7d210f4a8e/chrome/common/pdf_uma.cc [add] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/common/pdf_util.cc [rename] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/common/pdf_util.h [modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/renderer/chrome_content_renderer_client.cc [modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/renderer/plugins/pdf_plugin_placeholder.cc [modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/renderer/resources/plugins/pdf_plugin.html [add] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/test/data/pdf/pdf_embed_csp.html [add] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/test/data/pdf/pdf_iframe_csp.html
,
Oct 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8eb4c77037ba65b78883c5fca2b40411fb8c055c commit 8eb4c77037ba65b78883c5fca2b40411fb8c055c Author: Tommy C. Li <tommycli@chromium.org> Date: Mon Oct 15 20:39:52 2018 Click to Open PDF: Add IFRAME CSP browsertest. This CL is a followup to: https://chromium-review.googlesource.com/c/chromium/src/+/1273985 It adds the IFRAME test case, which did not seem flaky. The EMBED test case is still in-progress to deflake. Bug: 887752, 879149 , 878871 Change-Id: Iee52d4314539b2d457812a220a42051caeb1bab0 Reviewed-on: https://chromium-review.googlesource.com/c/1281262 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#599738} [modify] https://crrev.com/8eb4c77037ba65b78883c5fca2b40411fb8c055c/chrome/browser/pdf/pdf_extension_test.cc
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fac0d44345c4f0a7fc8f4f1464e7e79e1cb0e284 commit fac0d44345c4f0a7fc8f4f1464e7e79e1cb0e284 Author: Tommy C. Li <tommycli@chromium.org> Date: Thu Nov 15 16:56:48 2018 Plugins: Move WaitForPluginPlaceholder to a PluginTestUtils support Previously, only PluginPowerSaverBrowserTest had access to the ability to await the placeholder to be ready. Now that we want to do this for a PDF placeholder browser test as well, move this functionality to a test support class. This is patch 1/3 for this objective. Bug: 887752, 879149 , 878871 Change-Id: Ib0bc70f21d25ec35874025c0952f7c42555fce5b Reviewed-on: https://chromium-review.googlesource.com/c/1335781 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/master@{#608399} [modify] https://crrev.com/fac0d44345c4f0a7fc8f4f1464e7e79e1cb0e284/chrome/browser/BUILD.gn [modify] https://crrev.com/fac0d44345c4f0a7fc8f4f1464e7e79e1cb0e284/chrome/browser/plugins/plugin_power_saver_browsertest.cc [add] https://crrev.com/fac0d44345c4f0a7fc8f4f1464e7e79e1cb0e284/chrome/browser/plugins/plugin_test_utils.cc [add] https://crrev.com/fac0d44345c4f0a7fc8f4f1464e7e79e1cb0e284/chrome/browser/plugins/plugin_test_utils.h
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d89b22434f718dd0f82833b5a38ebc92444a99ea commit d89b22434f718dd0f82833b5a38ebc92444a99ea Author: Tommy C. Li <tommycli@chromium.org> Date: Thu Nov 15 16:57:52 2018 Plugins: Move NotifyPlaceholderReadyForTestingCallback to base class Move the LoadablePluginPlaceholder::DidFinishIconRepositionForTestingCallback method to the PluginPlaceholderBase base class so that the PDFPluginPlaceholder can also use it. This is a followup to: https://chromium-review.googlesource.com/c/chromium/src/+/1335781 This is patch 2/3 for this objective. Bug: 887752, 879149 , 878871 Change-Id: I14f2205d0d8d7c5fcd14fc2eac84e2b7fd1d52dc Reviewed-on: https://chromium-review.googlesource.com/c/1335997 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/master@{#608401} [modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/chrome/renderer/plugins/chrome_plugin_placeholder.cc [modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/chrome/renderer/plugins/pdf_plugin_placeholder.cc [modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/chrome/renderer/resources/plugins/blocked_plugin.html [modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/chrome/renderer/resources/plugins/pdf_plugin.html [modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/chrome/renderer/resources/plugins/plugin_poster.html [modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/chrome/renderer/resources/plugins/prefer_html_plugin.html [modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/components/plugins/renderer/loadable_plugin_placeholder.cc [modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/components/plugins/renderer/loadable_plugin_placeholder.h [modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/components/plugins/renderer/plugin_placeholder.cc [modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/components/plugins/renderer/plugin_placeholder.h
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c886c53eae3a8378502c4e185e29bfa709d4b97 commit 1c886c53eae3a8378502c4e185e29bfa709d4b97 Author: Tommy C. Li <tommycli@chromium.org> Date: Thu Nov 15 17:13:03 2018 Click to Open PDF: Add EMBED browsertest This re-adds the previously approved and committed browsertest, but this time with a fix for the flakiness. The body of the test remains the same, but with an extra call to: PluginTestUtils::WaitForPlaceholderReady(GetActiveWebContents(), "pdf_embed"); There is also an update the command line flags and an extra include. This is patch 3/3 for this objective. Bug: 887752, 879149 , 878871 Change-Id: Ie8c70d68fb3e3ebf9e70e5d9f2f1aa2f37b190f4 Reviewed-on: https://chromium-review.googlesource.com/c/1336086 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#608411} [modify] https://crrev.com/1c886c53eae3a8378502c4e185e29bfa709d4b97/chrome/browser/pdf/pdf_extension_test.cc [modify] https://crrev.com/1c886c53eae3a8378502c4e185e29bfa709d4b97/chrome/test/data/pdf/pdf_embed_csp.html |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by vamshi.kommuri@chromium.org
, Aug 30