"Cut" and "Copy" shouldn't be enabled when opening the page context menu on an input/textarea without a selection
Categories
(Firefox :: Menus, defect, P3)
Tracking
()
People
(Reporter: Gijs, Assigned: jfkthame)
References
(Blocks 1 open bug)
Details
(Keywords: helpwanted, Whiteboard: [proton-context-menus])
Attachments
(2 files)
UX has pointed out that we enable Cut and Copy even when there's no selection. This seems wrong.
Updated•4 years ago
|
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Comment 1•4 years ago
|
||
It is odd, but this was intentional. See bug 1159490 for more details. The gist is that rather than come up with a way for a page to indicate that cut/copy/paste should be enabled, they are just always enabled. This only applies to web pages though; our chrome ui should be correct.
Chrome (the browser) seems to have a bug here. It disables the context menu commands, but the menubar/keyboard shortcuts continue to be enabled.
Reporter | ||
Comment 2•4 years ago
|
||
(In reply to Neil Deakin from comment #1)
It is odd, but this was intentional. See bug 1159490 for more details. The gist is that rather than come up with a way for a page to indicate that cut/copy/paste should be enabled, they are just always enabled. This only applies to web pages though; our chrome ui should be correct.
Chrome (the browser) seems to have a bug here. It disables the context menu commands, but the menubar/keyboard shortcuts continue to be enabled.
Shouldn't we still disable them when context-clicking inside a plaintext <input>
or <textarea>
? How would the webpage do anything meaningful there based on cut/copy events? :-\
Comment 3•4 years ago
|
||
I don't think I could answer that. Maybe nika might?
Comment 4•4 years ago
|
||
(In reply to :Gijs (he/him) from comment #2)
Shouldn't we still disable them when context-clicking inside a plaintext
<input>
or<textarea>
? How would the webpage do anything meaningful there based on cut/copy events? :-\
We still fire clipboard events like the "copy" or "paste" event even if focus is in a normal plaintext <input>
or <textarea>
, and the webpage can intercept those and change what value you're copying, so the webpage can still meaningfully interact in this scenario.
As an example, if you load the data URI data:text/html,<input oncopy="event.clipboardData.setData('text/plain', 'hello, world!'); event.preventDefault();">
, right click in the empty textarea, and select "Copy", you'll notice that the string "hello, world!" has been copied to your clipboard.
Reporter | ||
Comment 5•4 years ago
•
|
||
(In reply to Nika Layzell [:nika] (ni? for response) from comment #4)
(In reply to :Gijs (he/him) from comment #2)
Shouldn't we still disable them when context-clicking inside a plaintext
<input>
or<textarea>
? How would the webpage do anything meaningful there based on cut/copy events? :-\We still fire clipboard events like the "copy" or "paste" event even if focus is in a normal plaintext
<input>
or<textarea>
, and the webpage can intercept those and change what value you're copying, so the webpage can still meaningfully interact in this scenario.As an example, if you load the data URI
data:text/html,<input oncopy="event.clipboardData.setData('text/plain', 'hello, world!'); event.preventDefault();">
, right click in the empty textarea, and select "Copy", you'll notice that the string "hello, world!" has been copied to your clipboard.
I understand that this is technically possible. My question was more about what would be a meaningful/reasonable use of this mechanism, if there is no selection anyway, and we're inside an inputfield.
It seems to me that the common case here is that there is no event handler, and so creating weird UX (enabled items that do nothing and aren't useful) for the uncommon/implausible case feels like the wrong decision.
If we're convinced this is important, could we use the event listener service to detect if a cut/copy listener exists, and disable the items when there's no selection and such a listener does not exist?
Comment 6•4 years ago
|
||
Yeah, we could potentially do something like that. I made the original change to force the options to always be available when I was an intern because we were not getting the option sometimes when we should, but if it's creating a bad UX we could definitely change it.
Something like checking for a cut
/copy
listener seems like it could be a reasonable approach if it's not too painful to implement.
Assignee | ||
Comment 7•4 years ago
|
||
I tried a small patch to handle this in TextEditor::IsCut[Copy]CommandEnabled()
, which seems to work OK in simple testing, at least. It checks for an event listener on the editor, but will not enable the commands if no text is selected but there's a cut/copy listener somewhere on an ancestor of the input element. Yet keyboard shortcuts will still send the command and the listener gets to handle it.
So this is arguably a bit inconsistent, but seems to me that it's a fairly obscure edge case, and probably doing the "right thing" in the simple case is worthwhile.
Anyhow, I'll post the patch for consideration.
Assignee | ||
Comment 8•4 years ago
|
||
Updated•4 years ago
|
Comment 10•4 years ago
|
||
smaug: Do you think that event listener should be check only editing host or its inclusive ancestors? See https://phabricator.services.mozilla.com/D107480#inline-600709
Updated•4 years ago
|
Updated•4 years ago
|
Comment 11•4 years ago
|
||
Pushed by jkew@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5b1ba8dbfc8a Avoid enabling Cut/Copy menu commands in text editor context when there's no selection, unless a listener is present. r=masayuki https://hg.mozilla.org/integration/autoland/rev/988b919adc5c Update tests for new Cut/Copy command-enabling behavior. r=masayuki
Comment 12•4 years ago
|
||
Backed out for causing build bustages on EditorBase.cpp
Backout link: https://hg.mozilla.org/integration/autoland/rev/ba7a0e46f822029f67ba53fc9adc670134656c0b
Failure log: https://treeherder.mozilla.org/logviewer?job_id=332827226&repo=autoland&lineNumber=13667
Comment 13•4 years ago
|
||
Pushed by jkew@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b88ab8c6dd39 Avoid enabling Cut/Copy menu commands in text editor context when there's no selection, unless a listener is present. r=masayuki https://hg.mozilla.org/integration/autoland/rev/dd24e610d956 Update tests for new Cut/Copy command-enabling behavior. r=masayuki
Comment 14•4 years ago
|
||
Backed out for failures on browser_contextmenu.js
backout: https://hg.mozilla.org/integration/autoland/rev/40869e2eef251908f942c9a35158e54579f0399d
failure log: https://treeherder.mozilla.org/logviewer?job_id=332888122&repo=autoland&lineNumber=2262
[task 2021-03-11T21:03:40.474Z] 21:03:40 INFO - TEST-PASS | browser/base/content/test/contextMenu/browser_contextmenu.js | checking item #0 (context-copy) name -
[task 2021-03-11T21:03:40.475Z] 21:03:40 INFO - Buffered messages finished
[task 2021-03-11T21:03:40.476Z] 21:03:40 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/contextMenu/browser_contextmenu.js | checking item #0 (context-copy) enabled state - Got false, expected true
[task 2021-03-11T21:03:40.476Z] 21:03:40 INFO - Stack trace:
[task 2021-03-11T21:03:40.476Z] 21:03:40 INFO - chrome://mochikit/content/browser-test.js:test_is:1359
[task 2021-03-11T21:03:40.476Z] 21:03:40 INFO - chrome://mochitests/content/browser/browser/base/content/test/contextMenu/contextmenu_common.js:checkMenuItem:241
[task 2021-03-11T21:03:40.476Z] 21:03:40 INFO - chrome://mochitests/content/browser/browser/base/content/test/contextMenu/contextmenu_common.js:checkMenu:305
[task 2021-03-11T21:03:40.476Z] 21:03:40 INFO - chrome://mochitests/content/browser/browser/base/content/test/contextMenu/contextmenu_common.js:checkContextMenu:175
[task 2021-03-11T21:03:40.476Z] 21:03:40 INFO - chrome://mochitests/content/browser/browser/base/content/test/contextMenu/contextmenu_common.js:test_contextmenu:444
[task 2021-03-11T21:03:40.478Z] 21:03:40 INFO - TEST-PASS | browser/base/content/test/contextMenu/browser_contextmenu.js | checking item #1 (context-selectall) name -
Updated•4 years ago
|
Comment 15•4 years ago
|
||
Also failing: https://treeherder.mozilla.org/logviewer?job_id=332898050&repo=autoland&lineNumber=1531
Assignee | ||
Comment 16•4 years ago
|
||
(In reply to Natalia Csoregi [:nataliaCs] from comment #14)
Backed out for failures on browser_contextmenu.js
Huh, this was a bit odd; the test is supposed to select the contents of a text div, so the Copy command would be enabled, but running it with added delays to make it easier to watch, it appears that the <input> element that's used earlier to check clipboard contents holds on to focus, and as none of its content is selected, it disables the Copy command.
Explicitly adding input.blur()
after we're finished with that element, so that it relinquishes focus before we continue with opening context menus elsewhere, results in the rest of the tests behaving as expected.
Comment 17•4 years ago
|
||
Pushed by jkew@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8148b9e5dfba Avoid enabling Cut/Copy menu commands in text editor context when there's no selection, unless a listener is present. r=masayuki https://hg.mozilla.org/integration/autoland/rev/0f6717c26ae3 Update tests for new Cut/Copy command-enabling behavior. r=masayuki
Comment 18•4 years ago
|
||
Pushed by malexandru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6060fa16bc3e Fix eslint prettier failures in browser_controller.js a=lint-fix
Comment 19•4 years ago
|
||
Backed out 3 changesets (bug 1692673) for bc failures in browser_contextmenu_input.js.
https://hg.mozilla.org/integration/autoland/rev/749a8350e2489721dfdbe9850ea4d17116eae66f
Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&revision=0f6717c26ae397929cc55f45a386913273cc49fc&selectedTaskRun=T-PVCFP0Q5uIw58HOiLJyg.0
Failure log:
https://treeherder.mozilla.org/logviewer?job_id=333003367&repo=autoland&lineNumber=3324
And also, the eslint fix:
https://treeherder.mozilla.org/logviewer?job_id=332998134&repo=autoland&lineNumber=472
Assignee | ||
Comment 20•4 years ago
|
||
Sigh... another test that assumes the old behavior where the commands are unconditionally enabled in edit fields. Not sure why that didn't show up on try; maybe it's different on Linux. Pushing a try job with more platforms to try and get it right next time....
Comment 21•4 years ago
|
||
Pushed by jkew@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/128681c5a081 Avoid enabling Cut/Copy menu commands in text editor context when there's no selection, unless a listener is present. r=masayuki https://hg.mozilla.org/integration/autoland/rev/06d6db9466df Update tests for new Cut/Copy command-enabling behavior. r=masayuki
Comment 22•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/128681c5a081
https://hg.mozilla.org/mozilla-central/rev/06d6db9466df
Comment 23•4 years ago
|
||
The patch landed in nightly and beta is affected.
:jfkthame, is this bug important enough to require an uplift?
If not please set status_beta
to wontfix
.
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 24•4 years ago
|
||
(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #23)
The patch landed in nightly and beta is affected.
:jfkthame, is this bug important enough to require an uplift?
I don't think so; this is a longstanding issue, and the fix is basically a cosmetic improvement to get the state of the menuitems to better reflect whether they're relevant.
Comment 26•4 years ago
|
||
This issue is verified fixed using Firefox 89.0a1 (BuildId:20210322174641) and Firefox 88.0b1 (BuildId:20210322185611) on Windows 10 64bit, macOS 10.14 and Ubuntu 20.04.
Description
•