New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 771604 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug



Sign in to add a comment

When Javascript selects text in an HTML element, Control-C can no longer be used to copy it to the clipboard

Reported by crow...@gmail.com, Oct 4 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36

Example URL:
http://conwaylife.com/forums/viewtopic.php?f=7&t=1994&start=75#p51150

Steps to reproduce the problem:
1. Navigate to http://conwaylife.com/forums/viewtopic.php?f=7&t=1994&start=75#p51150
2. Click on "Select All" in the box labelled "CODE" and then hit Ctrl+C to copy.

What is the expected behavior?
The selected text should be copied to the clipboard.

What went wrong?
The select appears to be made correctly, but Ctrl-C now has no effect. Previous clipboard contents are retained.

Does it occur on multiple sites: Yes

Is it a problem with a plugin? No 

Did this work before? Yes 59

Does this work in other browsers? Yes

Chrome version: 60.0.3112.113  Channel: stable
OS Version: 10.0
Flash Version: 

This works in other browsers.
 
Labels: Needs-Bisect Needs-Milestone

Comment 2 by woxxom@gmail.com, Oct 5 2017

Bisect info: 472976 (good) - 472989 (bad)
https://chromium.googlesource.com/chromium/src/+log/8a14c405..b3cad27b?pretty=fuller
Suspecting r472985 "Fix behavior of clipboard commands when there is unfocused selection"
Landed in 60.0.3104.0

A workaround for that forum script would be to explicitly focus the CODE element.
The problem, though, is that there are lots of forums that won't update or can't update their code so r472985 breaks thousands of such sites.
The new observed behavior is intended, but I don't understand what problem it solved and was it worth breaking popular legacy code.
Cc: vamshi.k...@techmahindra.com
Components: -Blink Blink>Editing
Labels: -Pri-2 -Needs-Bisect Triaged-ET M-63 hasbisect Pri-1
Owner: xiaoche...@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce this issue on 60.0.3112.113 , on latest canary 63.0.3232.0 using Mac 10.12.6, Windows 10 and Ubuntu 14.04 with steps mentioned in comment#0.

Manual Bisect Info:
================
Good Build: 60.0.3103.0
Bad Build: 60.0.3104.0

Note: Could not perform per-revision-bisect for this range, so did the chromium bisect
You are probably looking for a change made after 472976 (known good), but no later than 472989 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/8a14c40573ecd9fbf68ac4c4d97b9e4c9f504a6d..b3cad27b421b2aa9f6b0a89c80238480ba14023e

Review-Url: https://codereview.chromium.org/2894473003

Suspecting same from changelog.

@Xiaocheng: Please confirm the bug and help in re-assigning if it is not related to your change.

Thanks!
woxxom@: The behavior change is after we decoupled selection and focus. As a result, we allow focusing something else in the page (e.g., a link) without changing the selection, to reduce unnecessary selection changes.

It seems that we underestimated the existing dependency on the legacy behavior. We'll discuss how to deal with it next week.
Pada 7 Okt 2017 05:33, "xiaoche… via monorail" <
monorail+v2.1014188007@chromium.org> menulis:
Had an offline discussion with yosin@. Conclusions:

1. The spec of clipboard actions says nothing about focus (*). Hence, for interop, we should partially revert r472985 to stop checking focus when doing copy action

2. The reason why we don't go for full revert is that, 'cut' and 'paste' without focus seem more broken than 'copy', because they change the document content. We don't think changing unfocused content is the right behavior. We will file a spec bug about it.

(*) https://w3c.github.io/clipboard-apis/#clipboard-actions
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 12 2017

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

commit 75a47d437c69a18e12e6b419fbe9f4f527a74e29
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu Oct 12 03:33:32 2017

Remove focus check from copy action

This is a partial revert of https://codereview.chromium.org/2894473003
for copy. With this patch, we stop checking focus when doing a copy
action. Reasons:

1. Copying unfocused content still makes sense
2. Spec (*) doesn't require selection to have focus to be copied
3. A lot of sites rely on copying without focus

(*) https://w3c.github.io/clipboard-apis/#clipboard-actions

Bug:  771604 
Change-Id: I50caa4afb381f324cf86f168456e6bcecb24b082
Reviewed-on: https://chromium-review.googlesource.com/713742
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508247}
[modify] https://crrev.com/75a47d437c69a18e12e6b419fbe9f4f527a74e29/third_party/WebKit/LayoutTests/editing/pasteboard/pasteboard_with_unfocused_selection.html
[modify] https://crrev.com/75a47d437c69a18e12e6b419fbe9f4f527a74e29/third_party/WebKit/Source/core/editing/Editor.cpp
[modify] https://crrev.com/75a47d437c69a18e12e6b419fbe9f4f527a74e29/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp

Labels: Merge-Request-62
Requesting to merge to M62 since this seems to be an important interop issue.

If it get approved, I'll merge it on Fri evening PDT, after r508247 is in Canary for one day.
Project Member

Comment 9 by sheriffbot@chromium.org, Oct 12 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: We are only 4 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
this issue is resent in M60 and M61. I am quite nervous since this change has not been baked in canary yet and our next M62 release is stable. Can you please comment on how critical this merge is, how safe it is overall, and what is the full impact if we wait until M63?
abdulsyed@: Thanks for reviewing!

The issue is no longer in M60, as r472985 has been fully reverted in M60 due to  issue 741968 . It's current in M61 and M62 only.

I'm posting the merge request early so that we have more time to discuss, since there are not many working days before the M62 stable release. I won't proceed to merge before it's fully baked in Canary.

How critical: Critical, affecting all users doing the copy action on all platforms. And copy is very heavily used (~3%).

Although copy doesn't always fail, when it does fail, it can be very hard for a user/developer to reason because the success condition (selection has focus) isn't something intuitive, and is not in spec, either. To see the confusion, we have a bug report ( issue 752405 ) complaining that an minimal (according to spec) example of select-and-copy fails.

In fact, the same reason applies to the full revert of r472985 in M60, which happened after M60 stable release.

How safe: Should be pretty safe, since:
- The copy action doesn't use focus, so removing focus checking shouldn't cause any crash
- The full revert (for all of 'copy', 'cut' and 'paste') has been in the entire M60 without getting any complaint. As the three actions work independently, so it shouldn't break any functionality.

Full impact if wait until M63: M62 will appear as failing to copy in random situations (which might not be easy for a user to reason about), while other browsers succeed.
Cc: manoranj...@chromium.org
+manoranjanr@ 

Thanks for details - I agree this can be quite annoying for users. Let's wait until Canary results and we can re-review tomorrow. 

I think it would be good to get TE verification for this as well in Canary.  
Labels: ReleaseBlock-Stable M-62
Cc: brajkumar@chromium.org
Labels: OS-Linux OS-Mac
Verified this issue on Ubuntu 14.04 and Mac OS 10.12.6 using chrome latest canary #63.0.3239.0 by following steps mentioned in the original comment. Observed able to copy the code to clipboard and able to paste the content as expected. Hence adding TE-Verified label for M-63.

Note: Not verified on Win-pgo builds, but verified on non Win-pgo builds issues seems to be resolved. Will verify on Win-pgo once the canary build is available in the market.

Thanks!
Labels: -Merge-Review-62 Merge-Approved-62
Approving merge to M62. Branch:3202
Thanks for the review and the verification!
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 13 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a80cec8598e9253f4e0223f01064be6fab7cd6c

commit 6a80cec8598e9253f4e0223f01064be6fab7cd6c
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Oct 13 23:21:20 2017

Remove focus check from copy action

This is a partial revert of https://codereview.chromium.org/2894473003
for copy. With this patch, we stop checking focus when doing a copy
action. Reasons:

1. Copying unfocused content still makes sense
2. Spec (*) doesn't require selection to have focus to be copied
3. A lot of sites rely on copying without focus

(*) https://w3c.github.io/clipboard-apis/#clipboard-actions

TBR=xiaochengh@chromium.org

(cherry picked from commit 75a47d437c69a18e12e6b419fbe9f4f527a74e29)

Bug:  771604 
Change-Id: I50caa4afb381f324cf86f168456e6bcecb24b082
Reviewed-on: https://chromium-review.googlesource.com/713742
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#508247}
Reviewed-on: https://chromium-review.googlesource.com/720077
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#685}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/6a80cec8598e9253f4e0223f01064be6fab7cd6c/third_party/WebKit/LayoutTests/editing/pasteboard/pasteboard_with_unfocused_selection.html
[modify] https://crrev.com/6a80cec8598e9253f4e0223f01064be6fab7cd6c/third_party/WebKit/Source/core/editing/Editor.cpp
[modify] https://crrev.com/6a80cec8598e9253f4e0223f01064be6fab7cd6c/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp

Status: Fixed (was: Assigned)

Comment 19 Deleted

Comment 20 by hu...@vewd.com, Dec 4 2017

 Issue 752405  has been merged into this issue.

Sign in to add a comment