1100562 - Console - right click on entry, "Copy" is disabled

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Screenshot of bug

See attached screenshot.

STR:
Open the Browser Console
Right click on an entry

ER:
The "Copy" option is enabled and allows for copying the entry to the clipboard.

AR:
The "Copy" option is disabled.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Also broken for non-Browser console.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Looks like you have to make a selection to enable "Copy". We should allow copy to work without a selection, by placing the contents of the targeted entry in to the clipboard.

Comment 3

[Athena]

Attachment: console-copy-bug-1100562.patch

* if we have no text selected when we right-click on a message, let us copy the entire message

* if we have some text selected, don't handle it here; fall back on default copy behavior

* removed timestamp from copySelectedItems, because it's present in (item.clipboardText (this shouldn't affect anything else because the only other caller is in link-only mode)

Comment 4

[Mihai Sucan [:msucan]]

Comment on attachment 8527277 [details] [diff] [review]
console-copy-bug-1100562.patch

Review of attachment 8527277 [details] [diff] [review]:
-----------------------------------------------------------------

Athena, thanks for your patch. This is really good, close to r+.

::: browser/devtools/webconsole/test/browser.ini
@@ +127,5 @@
>    test_bug_1010953_cspro.html
>    test_bug1045902_console_csp_ignore_reflected_xss_message.html^headers^
>    test_bug1045902_console_csp_ignore_reflected_xss_message.html
>  
> +[browser_bug_1100562_copy_message.js]

We should try to avoid bug numbers. I suggest browser_console_copy_entire_message_context_menu.js.

::: browser/devtools/webconsole/test/browser_bug_1100562_copy_message.js
@@ +2,5 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +const TEST_URI = "http://example.com/browser/browser/devtools/webconsole/test/test-console.html";

Please add a short comment above explaining what this test does.

@@ +10,5 @@
> +let HUD = null;
> +let outputNode = null;
> +let contextMenu = null;
> +
> +function test() {

This test is fine however please change it to use the new style of writing tests. See browser_webconsole_console_api_stackframe.js for an example - please follow the style as much as possible.

@@ +47,5 @@
> +
> +  message.scrollIntoView();
> +  waitForContextMenu(contextMenu, message, () => {
> +    let copyItem = contextMenu.querySelector(CONTEXT_MENU_ID);
> +    copyItem.doCommand();

You should also check if copyItem is enabled.

::: browser/devtools/webconsole/webconsole.js
@@ +4797,5 @@
>        }
> +      case "cmd_copy": {
> +        // Only copy if there's no selected text.
> +        // With text selected, we want to fall back onto the default copy behavior.
> +        return !this.owner.output.getSelectedMessages(1)[0];

We should also check if lastClickedMessage is not null, see the consoleCmd_copyURL command.

Comment 5

[Athena]

Attachment: console-copy-bug-1100562.patch

Comment 6

[Mihai Sucan [:msucan]]

Comment on attachment 8530792 [details] [diff] [review]
console-copy-bug-1100562.patch

Review of attachment 8530792 [details] [diff] [review]:
-----------------------------------------------------------------

Re-assigning to Panos.

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8530792 [details] [diff] [review]
console-copy-bug-1100562.patch

Review of attachment 8530792 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch! r=me with the changes below.

::: browser/devtools/webconsole/test/browser.ini
@@ +144,5 @@
>  [browser_console_click_focus.js]
>  [browser_console_consolejsm_output.js]
>  [browser_console_dead_objects.js]
>  [browser_console_error_source_click.js]
> +[browser_console_copy_entire_message_context_menu.js]

This test needs to go above the browser_console_error_source_click.js entry, as the following skip-if line should still apply to that. This hunk needs rebasing anyway, so while you do that, fix this as well.

::: browser/devtools/webconsole/test/head.js
@@ +261,5 @@
>   *        Function to invoke on popuphidden event.
>   */
>  function waitForContextMenu(aPopup, aButton, aOnShown, aOnHidden)
>  {
> +  let deferred = promise.defer();

All the changes to head.js are no longer necessary, they have been made in a previous patch.

Comment 8

[Athena]

Attachment: console-copy-bug-1100562.patch

Thanks for the review!

try run passed: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=aa20d6e7268f

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/3e3ad121d5d7

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3e3ad121d5d7

Comment 12

[Ashwini K H]

Bug Verification Day-2015-14-01

Comment 13

[Ashwini K H]

Bug Verification Day-2015-14-01

Comment 14

[[e-mail]]

Bug Verified, still persists in my Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:38.0) Gecko/20100101 Firefox/38.0
copy is disabled first time I click on any place in my browser, second click on anywhere enables the copy function

Comment 15

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

(In reply to [e-mail] from comment #14)
> Bug Verified, still persists in my Mozilla/5.0 (Macintosh; Intel Mac OS X
> 10.10; rv:38.0) Gecko/20100101 Firefox/38.0
> copy is disabled first time I click on any place in my browser, second click
> on anywhere enables the copy function

Thank you for verifying this roxrockers! The issue you are reporting with Copy disabled on first right-click is tracked separately in bug 1158474.