Closed Bug 1524115 Opened 8 months ago Closed 2 months ago

When clicking and selecting source text on a collapsible message, the message expansion is toggled after releasing the mouse

Categories

(DevTools :: Console, defect, P3)

defect

Tracking

(firefox70 fixed)

RESOLVED FIXED
Firefox 70
Tracking Status
firefox70 --- fixed

People

(Reporter: bgrins, Assigned: armando.ferreira, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(4 files)

STR:

  • execute console.group("foo"); console.log("bar"); console.groupEnd("foo")
  • select and highlight the source (in this case, "debugger eval code:1:1")
  • let go of mouse

Expected:
"debugger eval code:1:1" is selected, group is still open

Actual:
"debugger eval code:1:1" is selected, but group is closed

If you select again then the group gets reopened. I'd expect the click and drag to not toggle the group as a normal click does.

The Message toggling happens in devtools/client/webconsole/components/Message.js#111-118, and is fired on click.

We should do an early return if there's any text selected.
It could look something like:

const selection = e.target.ownerDocument.defaultView.getSelection();
if (selection.type === "Range") {
  return;
}

Then we could have a test similar to devtools/client/webconsole/test/mochitest/browser_webconsole_object_inspector_selected_text.js

I'd be happy to mentor anyone who'd like to work on this 🙂

Mentor: nchevobbe
Keywords: good-first-bug
Priority: -- → P3

Hey, can I take this up?

Hello srestha, thanks for wanting to help us.

You can set up the work environment by following this page https://docs.firefox-dev.tools/getting-started/
Also, feel free to ask any question, here or on our Slack

Assignee: nobody → sresthasrivastava.ss
Attached image toggle-on-select.gif

Here's another example where this makes it hard to copy out a stack trace. Looks like the proposed fix will cover this case as well.

Hello srestha!
Is there anything blocking you on this bug?
No worries if you simply don't have the time to look into this :)

Flags: needinfo?(sresthasrivastava.ss)

Hey, sorry for the late reply, I just got busy, I'll get it done in a day or two.

Flags: needinfo?(sresthasrivastava.ss)

Hey, I don't quite understand the actual result, could someone upload a gif of it?

Srestha, you can look at the gif in comment 4 🙂

Status: NEW → ASSIGNED

Hello again Srestha.
Do you still plan to work on this? Is there anything I can clarify?

Flags: needinfo?(sresthasrivastava.ss)

clearing assignee

Assignee: sresthasrivastava.ss → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(sresthasrivastava.ss)

Hi Nicolas, Can I also take this?

Flags: needinfo?(nchevobbe)

Hello Armando, it's yours :)

Assignee: nobody → armando.ferreira
Status: NEW → ASSIGNED
Flags: needinfo?(nchevobbe)
Attached image testing-armando.gif

Hi Nicholas, I'm able to confirm that your proposed fix works like a charm, but I'm struggling to reproduce it on a test.

I created this new function in head.js to be able to select multiple nodes.

function selectNodes(hud, nodes) {
  const outputContainer = hud.ui.outputNode.querySelector(".webconsole-output");

  // We must first blur the input or else we can't select anything.
  outputContainer.ownerDocument.activeElement.blur();

  const selection = outputContainer.ownerDocument.getSelection();
  selection.removeAllRanges();

  for (const node of nodes) {
    const range = document.createRange();
    range.selectNodeContents(node);
    selection.addRange(range)
  }

  return selection;
}

But when using it like this on a test:

add_task(async function () {

  const hud = await openNewTabAndConsole(TEST_URI);
  const label = "bat";

  const onLoggedMessage = waitForMessage(hud, label);

  await ContentTask.spawn(gBrowser.selectedBrowser, label, function (str) {
    content.wrappedJSObject.console.group(str, "collapsible-test");
    content.wrappedJSObject.console.log("joy");
    content.wrappedJSObject.console.log("can");
    content.wrappedJSObject.console.log("sip");
    content.wrappedJSObject.console.log("van");
    content.wrappedJSObject.console.groupEnd("collapsible-test");
  });
  const { node } = await onLoggedMessage;

  info(`Select all the text in collapsible message`);
  selectNodes(hud, node.parentNode.querySelectorAll(".message"));
  await waitFor(() => 1 === 0);
});

I got the result as the GIF I uploaded in comment #14

Any suggestions? On how I can reproduce this issue on a test?

Flags: needinfo?(nchevobbe)

Hello Armando,

Despite my comment earlier, I don't think we can use the same test mechanism as things are a bit different.
Here, the group collapses because it register a click event at some point (because we mousedown, move the mouse, and mouseup, the down/up is what gives a click).
So what we should do is simulate this behavior.

EventUtils.sendMouseEvent({ type: "mousedown" }, node);
EventUtils.sendMouseEvent({ type: "mousemove", /* additional properties to actually move */ }, node);
EventUtils.sendMouseEvent({ type: "mouseup" }, node);

but I'm not sure this would work, as those events are simulated and might only impact the js, not the browser behavior
Maybe you can try to use selectNode between the mousedown and the mouseup.
Let me know if this work

Flags: needinfo?(nchevobbe)

Thanks Nicolas! I will try to reproduce with the sendMouseEvent()

When clicking and selecting source text on a collapsible message, the
message expansion is toggled after releasing the mouse

Hi Nicholas, I tried my best to reproduce the bug under a test but I was unable to do it. I tried sendMouseEvent with mousedown, mousemove, mouseup and using selectNode in between them but it was not being triggered. Also I tried dragEvents to see if this will trigger but it also didn't trigger the bug. I'm sending the fix to code review D37607.

Let me know if there is anything more I can do :)

Flags: needinfo?(nchevobbe)

I'll ask around and look if there's a way we can mimick the text selection, but otherwise we can drop it.
Were you waiting a bit between these operations? (like await new Promise(res => setTimeout(res, 10))), just to see if that would help?

Flags: needinfo?(nchevobbe)
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f7ca971deb77
When clicking and selecting source text on a collapsible message, the message expansion is toggled after releasing the mouse. r=nchevobbe
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
QA Whiteboard: [qa-70b-p2]
You need to log in before you can comment on or make changes to this bug.