New issue
Advanced search Search tips

Issue 591071 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Right click > Save As for PDF inside frame saves web page rather than PDF file

Project Member Reported by jbroman@chromium.org, Mar 1 2016

Issue description

Version: 48.0.2564.116 (Chrome stable)
OS: All

What steps will reproduce the problem?
1. Navigate to a site that embeds a PDF within a frame. For instance, TD WebBroker does this for its tax slips (under eServices)
2. Right-click.
3. Choose "Save As...".
4. Notice that the suggested filename is "WebBroker.html" and the type is "Webpage, Complete".

What is the expected output? What do you see instead?
I would expect to see a PDF filename and a PDF type, because I want to save a PDF file, not a frameset page. (I _can_ save the PDF by using the toolbar button which looks like the Gmail archive icon, however. Printing from the right-click menu also seems to work)

Looking more closely, it appears that TD opens a window, which loads a servlet page that contains a <frameset>, within which is the PDF file.

Reduced repro:
data:text/html,<frameset><frame src="http://www.cra-arc.gc.ca/E/pbg/tf/5000-s1/5000-s1-15e.pdf">
 
Summary: Right click > Save As for PDF inside frame saves web page rather than PDF file (was: Right click > Save As for PDF inside frame saves web archive rather than PDF file)
Status: Available (was: Untriaged)
Cc: lazyboy@chromium.org
lazyboy: Can you help take a quick peek at the IDC_SAVE_PAGE code in chrome/browser/renderer_context_menu/render_view_context_menu.cc which does:

embedder_web_contents_->OnSavePage();

and make sure that's the expected WebContents to talk to? Comments/suggestions are welcome.
I think that was mistakenly changed to embedder_web_contents_, it should be source_web_contents_.
lazyboy: I tried that but it doesn't seem to make a difference.
Cc: wjmaclean@chromium.org
Ya, that also breaks the working full page case.

The issue might be how WebContentsImpl::contents_mime_type_ is set. For full page case:
embedder_web_contents_.contents_mime_type_ = 'application/pdf'
source_web_contents_.contents_mime_type_ = 'text/html'

whereas for embedded pdf, both WebContent's contents_mime_type_ stays 'text/html'.
I don't know about the pdf saving code path at all, +James in case he has any idea.

Comment 7 by weili@chromium.org, Apr 29 2017

Owner: weili@chromium.org
Status: Started (was: Available)
I think fixing it makes sense since "print..." only print the embedded PDF instead of the whole doc.
Project Member

Comment 8 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee

commit fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee
Author: weili <weili@chromium.org>
Date: Fri May 19 06:04:09 2017

Save pdf file instead of web page through context menu for embedded pdf file

When user clicks 'Save as...' in the context menu over an embedded pdf file,
we should save the pdf file instead of the entire web page. We fix this by showing
the correct context menu for embedded pdf versus full page pdf: only full page
pdf's context menu shows page navigation items such as 'Reload',
but embedded pdf's context menu doesn't show them.

BUG= chromium:591071 

Review-Url: https://codereview.chromium.org/2862583010
Cr-Commit-Position: refs/heads/master@{#473105}

[modify] https://crrev.com/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee/chrome/browser/renderer_context_menu/render_view_context_menu.cc
[modify] https://crrev.com/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
[add] https://crrev.com/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee/chrome/test/data/pdf/test-embed-pdf.html
[add] https://crrev.com/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee/chrome/test/data/pdf/test-iframe-pdf.html

Project Member

Comment 9 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab6a896a5431f19e96cbc15ce7920730ac4ace51

commit ab6a896a5431f19e96cbc15ce7920730ac4ace51
Author: treib <treib@chromium.org>
Date: Fri May 19 12:58:49 2017

Revert of Save pdf file instead of web page through context menu for embedded pdf file (patchset #8 id:160001 of https://codereview.chromium.org/2862583010/ )

Reason for revert:
PdfPluginContextMenuBrowserTest.EmbeddedPdfHasNoPageItems is flaky on Windows, see:
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=browser_tests%20(with%20patch)&tests=PdfPluginContextMenuBrowserTest.EmbeddedPdfHasNoPageItems

Original issue's description:
> Save pdf file instead of web page through context menu for embedded pdf file
>
> When user clicks 'Save as...' in the context menu over an embedded pdf file,
> we should save the pdf file instead of the entire web page. We fix this by showing
> the correct context menu for embedded pdf versus full page pdf: only full page
> pdf's context menu shows page navigation items such as 'Reload',
> but embedded pdf's context menu doesn't show them.
>
> BUG= chromium:591071 
>
> Review-Url: https://codereview.chromium.org/2862583010
> Cr-Commit-Position: refs/heads/master@{#473105}
> Committed: https://chromium.googlesource.com/chromium/src/+/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee

TBR=lazyboy@chromium.org,weili@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= chromium:591071 

Review-Url: https://codereview.chromium.org/2891923005
Cr-Commit-Position: refs/heads/master@{#473171}

[modify] https://crrev.com/ab6a896a5431f19e96cbc15ce7920730ac4ace51/chrome/browser/renderer_context_menu/render_view_context_menu.cc
[modify] https://crrev.com/ab6a896a5431f19e96cbc15ce7920730ac4ace51/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
[delete] https://crrev.com/3e0003f69bab6a934d2a964248b20ba14b4e9c49/chrome/test/data/pdf/test-embed-pdf.html
[delete] https://crrev.com/3e0003f69bab6a934d2a964248b20ba14b4e9c49/chrome/test/data/pdf/test-iframe-pdf.html

Project Member

Comment 10 by bugdroid1@chromium.org, May 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/85a3174dfc2310377872a88db75f7370fa2f9734

commit 85a3174dfc2310377872a88db75f7370fa2f9734
Author: weili <weili@chromium.org>
Date: Mon May 22 18:12:33 2017

Remove the flaky test as the flaky test is not really necessary.

>Save pdf file instead of web page through context menu for embedded pdf file

>When user clicks 'Save as...' in the context menu over an embedded pdf file,
>we should save the pdf file instead of the entire web page. We fix this by showing
>the correct context menu for embedded pdf versus full page pdf: only full page
>pdf's context menu shows page navigation items such as 'Reload',
>but embedded pdf's context menu doesn't show them.

>BUG= chromium:591071 

>Review-Url: https://codereview.chromium.org/2862583010
>Cr-Commit-Position: refs/heads/master@{#473105}
>Committed: >https://chromium.googlesource.com/chromium/src/+/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee

Review-Url: https://codereview.chromium.org/2862583010
Cr-Commit-Position: refs/heads/master@{#473625}

[modify] https://crrev.com/85a3174dfc2310377872a88db75f7370fa2f9734/chrome/browser/renderer_context_menu/render_view_context_menu.cc
[modify] https://crrev.com/85a3174dfc2310377872a88db75f7370fa2f9734/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
[add] https://crrev.com/85a3174dfc2310377872a88db75f7370fa2f9734/chrome/test/data/pdf/test-iframe-pdf.html

Comment 11 by weili@chromium.org, May 23 2017

Status: Fixed (was: Started)

Sign in to add a comment