New issue
Advanced search Search tips

Issue 737585 link

Starred by 2 users
Status: Started
Owner:
wjmaclean@chromium.org
Cc:
dsinclair@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 2
Type: Feature

Blocked on:
issue 490184
issue 773265
issue 773276
issue 773669
issue 776839
issue 777851


Sign in to add a comment

Implement Touch Selection Editing for PDFs

Project Member Reported by wjmaclean@chromium.org, Jun 28 2017 
At present TouchSelectionEditing is not available for PDFs.

Implementing this will require:
- plumb selection actions in/out of PDFium via its PPAPI interface
- add a TouchSelectionControllerClient and TouchSelectionMenuClient for PDFs in the chrome/ layer
- register this client with content/ via RenderWidgetHostView
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 29 2017 

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

commit d70306a7bfbc29ff12d4469e92a9e2a3bf71e551
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Thu Jun 29 01:47:43 2017

Expose TouchSelectionControllerClientManager* on RenderWidgetHostView

This is the second of two refactoring CLs to allow creating
TouchSelectionControllerClients outside of content/ and
subsequently register them for use with the top level
RenderWidgetHostView.

This CL also renames the accessor, to make it conform to the
style guide rules for virtual functions.

Bug: 737585
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I2fffcd155d23dedfd08a987b501680f6b98035eb
Reviewed-on: https://chromium-review.googlesource.com/552580
Commit-Queue: James Maclean <wjmaclean@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483243}
[modify] https://crrev.com/d70306a7bfbc29ff12d4469e92a9e2a3bf71e551/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/d70306a7bfbc29ff12d4469e92a9e2a3bf71e551/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/d70306a7bfbc29ff12d4469e92a9e2a3bf71e551/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/d70306a7bfbc29ff12d4469e92a9e2a3bf71e551/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/d70306a7bfbc29ff12d4469e92a9e2a3bf71e551/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/d70306a7bfbc29ff12d4469e92a9e2a3bf71e551/content/public/browser/render_widget_host_view.h
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 10 2017 

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

commit 8408a5df240e70c1e37e503a5607c107a1728aa9
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Mon Jul 10 15:52:30 2017

Introduce TouchSelection Editing Interfaces for PDF

This CL introduces the interfaces TouchSelectionControllerClient and
TouchSelectionMenuClient, and the basic logic to support (initially)
copy on touch-selected content. It is not functional until the
required plumbing to export selection updates from PDFium lands in a
separate CL.

Bug: 737585
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Iaa5ed68a0590d3e2e27e5d2ae3d7b86ab5342e92
Reviewed-on: https://chromium-review.googlesource.com/558771
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485281}
[modify] https://crrev.com/8408a5df240e70c1e37e503a5607c107a1728aa9/components/pdf/DEPS
[modify] https://crrev.com/8408a5df240e70c1e37e503a5607c107a1728aa9/components/pdf/browser/BUILD.gn
[modify] https://crrev.com/8408a5df240e70c1e37e503a5607c107a1728aa9/components/pdf/browser/pdf_web_contents_helper.cc
[modify] https://crrev.com/8408a5df240e70c1e37e503a5607c107a1728aa9/components/pdf/browser/pdf_web_contents_helper.h
[modify] https://crrev.com/8408a5df240e70c1e37e503a5607c107a1728aa9/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/8408a5df240e70c1e37e503a5607c107a1728aa9/content/browser/frame_host/render_widget_host_view_guest.h

Comment 3 by pnangunoori@chromium.org, Jul 18 2017

Labels: Needs-Feedback
W. James MacLean -- Could you please provide the Sample PDF files and test steps to test this issue.

Thanks.

Comment 4 by wjmaclean@chromium.org, Jul 18 2017 

There is still one more CL to land (from dsinclair@) before there will be anything testable.
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 24 2017 

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

commit 9218acc1309c18e11e255dd99cfb282140cc5e62
Author: Dan Sinclair <dsinclair@chromium.org>
Date: Mon Jul 24 21:03:09 2017

Add touch selection for PDF files.

This CL plumbs through the need methods to allow touch selection with
the Copy action in PDF files.

Bug:  chromium:490184 , chromium:737585
Change-Id: I4dab62833dac94b6c8ed59cbcc7db8e9387a7fde
Reviewed-on: https://chromium-review.googlesource.com/565899
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489071}
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/components/pdf/browser/pdf_web_contents_helper.cc
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/components/pdf/browser/pdf_web_contents_helper.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/components/pdf/common/BUILD.gn
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/components/pdf/common/pdf.mojom
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/components/pdf/renderer/DEPS
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/components/pdf/renderer/pepper_pdf_host.cc
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/components/pdf/renderer/pepper_pdf_host.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/content/public/renderer/pepper_plugin_instance.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/content/renderer/pepper/fake_pepper_plugin_instance.cc
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/content/renderer/pepper/fake_pepper_plugin_instance.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/content/renderer/pepper/pepper_plugin_instance_impl.cc
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/content/renderer/pepper/pepper_plugin_instance_impl.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/pdf/out_of_process_instance.cc
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/pdf/out_of_process_instance.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/pdf/pdf_engine.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/pdf/pdfium/pdfium_engine.cc
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/pdf/pdfium/pdfium_engine.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/ppapi/c/private/ppb_pdf.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/ppapi/c/private/ppp_pdf.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/ppapi/cpp/private/pdf.cc
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/ppapi/cpp/private/pdf.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/ppapi/proxy/pdf_resource.cc
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/ppapi/proxy/pdf_resource.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/ppapi/proxy/pdf_resource_unittest.cc
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/ppapi/proxy/ppapi_messages.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/ppapi/proxy/ppp_pdf_proxy.cc
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/ppapi/proxy/ppp_pdf_proxy.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/ppapi/thunk/ppb_pdf_api.h
[modify] https://crrev.com/9218acc1309c18e11e255dd99cfb282140cc5e62/ppapi/thunk/ppb_pdf_thunk.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 25 2017 

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

commit 0d7094b3c580b9da58028691e5936026a2b3ed91
Author: Pavel Kalinnikov <pkalinnikov@chromium.org>
Date: Tue Jul 25 10:15:14 2017

Revert "Add touch selection for PDF files."

This reverts commit 9218acc1309c18e11e255dd99cfb282140cc5e62.

Reason for revert: The PDFResourceTest.SelectionChanged test introduced by this CL fails consistently on "Linux MSan Tests" and "Linux Chromium MSan Tests" bots.

Original change's description:
> Add touch selection for PDF files.
> 
> This CL plumbs through the need methods to allow touch selection with
> the Copy action in PDF files.
> 
> Bug:  chromium:490184 , chromium:737585
> Change-Id: I4dab62833dac94b6c8ed59cbcc7db8e9387a7fde
> Reviewed-on: https://chromium-review.googlesource.com/565899
> Reviewed-by: Lei Zhang <thestig@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Raymes Khoury <raymes@chromium.org>
> Reviewed-by: James MacLean <wjmaclean@chromium.org>
> Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#489071}

TBR=raymes@chromium.org,thestig@chromium.org,dsinclair@chromium.org,jam@chromium.org,tsepez@chromium.org,wjmaclean@chromium.org

Change-Id: I66c58582a3a14ad93ecf29e743de1071c44d19f0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:490184 , chromium:737585
Reviewed-on: https://chromium-review.googlesource.com/584613
Reviewed-by: Pavel Kalinnikov <pkalinnikov@chromium.org>
Commit-Queue: Pavel Kalinnikov <pkalinnikov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489270}
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/components/pdf/browser/pdf_web_contents_helper.cc
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/components/pdf/browser/pdf_web_contents_helper.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/components/pdf/common/BUILD.gn
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/components/pdf/common/pdf.mojom
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/components/pdf/renderer/DEPS
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/components/pdf/renderer/pepper_pdf_host.cc
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/components/pdf/renderer/pepper_pdf_host.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/content/public/renderer/pepper_plugin_instance.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/content/renderer/pepper/fake_pepper_plugin_instance.cc
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/content/renderer/pepper/fake_pepper_plugin_instance.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/content/renderer/pepper/pepper_plugin_instance_impl.cc
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/content/renderer/pepper/pepper_plugin_instance_impl.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/pdf/out_of_process_instance.cc
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/pdf/out_of_process_instance.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/pdf/pdf_engine.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/pdf/pdfium/pdfium_engine.cc
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/pdf/pdfium/pdfium_engine.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/ppapi/c/private/ppb_pdf.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/ppapi/c/private/ppp_pdf.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/ppapi/cpp/private/pdf.cc
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/ppapi/cpp/private/pdf.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/ppapi/proxy/pdf_resource.cc
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/ppapi/proxy/pdf_resource.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/ppapi/proxy/pdf_resource_unittest.cc
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/ppapi/proxy/ppapi_messages.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/ppapi/proxy/ppp_pdf_proxy.cc
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/ppapi/proxy/ppp_pdf_proxy.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/ppapi/thunk/ppb_pdf_api.h
[modify] https://crrev.com/0d7094b3c580b9da58028691e5936026a2b3ed91/ppapi/thunk/ppb_pdf_thunk.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 25 2017 

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

commit f50e9f3557742f92ff33521a24988578ba8fa88f
Author: Dan Sinclair <dsinclair@chromium.org>
Date: Tue Jul 25 21:06:41 2017

Reland "Add touch selection for PDF files.""

This reverts commit 0d7094b3c580b9da58028691e5936026a2b3ed91.

> Add touch selection for PDF files.
>
> This CL plumbs through the need methods to allow touch selection with
> the Copy action in PDF files.
>
> Bug:  chromium:490184 , chromium:737585
> Change-Id: I4dab62833dac94b6c8ed59cbcc7db8e9387a7fde
> Reviewed-on: https://chromium-review.googlesource.com/565899
> Reviewed-by: Lei Zhang <thestig@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Raymes Khoury <raymes@chromium.org>
> Reviewed-by: James MacLean <wjmaclean@chromium.org>
> Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#489071}

TBR=jam@chromium.org,tsepez@chromium.org,raymes@chromium.org

Bug:  chromium:490184 , chromium:737585,  chromium:748390 
Change-Id: Ief608baf13b1a5c04e19c9d7c2e7398e8d1c8cb5
Reviewed-on: https://chromium-review.googlesource.com/585107
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489429}
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/components/pdf/browser/pdf_web_contents_helper.cc
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/components/pdf/browser/pdf_web_contents_helper.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/components/pdf/common/BUILD.gn
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/components/pdf/common/pdf.mojom
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/components/pdf/renderer/DEPS
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/components/pdf/renderer/pepper_pdf_host.cc
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/components/pdf/renderer/pepper_pdf_host.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/content/public/renderer/pepper_plugin_instance.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/content/renderer/pepper/fake_pepper_plugin_instance.cc
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/content/renderer/pepper/fake_pepper_plugin_instance.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/content/renderer/pepper/pepper_plugin_instance_impl.cc
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/content/renderer/pepper/pepper_plugin_instance_impl.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/pdf/out_of_process_instance.cc
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/pdf/out_of_process_instance.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/pdf/pdf_engine.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/pdf/pdfium/pdfium_engine.cc
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/pdf/pdfium/pdfium_engine.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/ppapi/c/private/ppb_pdf.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/ppapi/c/private/ppp_pdf.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/ppapi/cpp/private/pdf.cc
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/ppapi/cpp/private/pdf.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/ppapi/proxy/pdf_resource.cc
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/ppapi/proxy/pdf_resource.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/ppapi/proxy/pdf_resource_unittest.cc
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/ppapi/proxy/ppapi_messages.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/ppapi/proxy/ppp_pdf_proxy.cc
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/ppapi/proxy/ppp_pdf_proxy.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/ppapi/thunk/ppb_pdf_api.h
[modify] https://crrev.com/f50e9f3557742f92ff33521a24988578ba8fa88f/ppapi/thunk/ppb_pdf_thunk.cc

Comment 8 by omrilio@chromium.org, Oct 1 2017 

Just to make sure I understand "TouchSelectionEditing " correctly. Once this bug is fixed, will this allow copying text in touch/tablet mode?

Comment 9 Deleted

Comment 10 by wjmaclean@chromium.org, Oct 2 2017 

Re C#9, yes.

At present 'copy' operations are supported, but cut/paste are waiting on PDFium to complete some work on forms editing.

Comment 11 by wjmaclean@chromium.org, Oct 10 2017

Blockedon: 773265 773276

Comment 12 by wjmaclean@chromium.org, Oct 11 2017

Blockedon: 773669

Comment 13 by wjmaclean@chromium.org, Oct 23 2017

Blockedon: 776839

Comment 14 by wjmaclean@chromium.org, Oct 24 2017

Blockedon: 777851
►

Sign in to add a comment