1187582 - Opening a link in markup view attributes triggers drag action

Status: VERIFIED FIXED

(DevTools :: Inspector, defect)

Description

[Sebastian Zartner [:sebo]]

When you middle-click or Ctrl+click a URL within the inspector's markup view, the link is opened in a new tab. When you then switch back to the previous tab (without closing the newly opened tab) and move the mouse into the inspector you see that you're dragging the just clicked tag.

Test case:
1. Inspect the 'More Features' link at https://getfirebug.com/.
2. Middle-click its URL path "/whatisfirebug" within the inspector
   => A new tab will be opened in the foreground showing "https://getfirebug.com/whatisfirebug". (OK)
3. Switch back to the preview tab
4. Move the mouse over the markup view of the inspector

=> The tag <a href="/whatisfirebug">More Features »</a> will be dragged.

Sebastian

Comment 1

[Brian Grinstead [:bgrins]]

Yep, I can confirm this issue.  We will need to get it fixed before 8-10 when we ship 42.  John, do you think you'll have a chance to look at this bug or should we assign it to someone else?

Comment 2

[John Pavlicek]

I can look into it, no problem.

Comment 3

[Brian Grinstead [:bgrins]]

Thanks!

Comment 4

[John Pavlicek]

Attachment: b1187582.patch

Quick patch to stop the drag.  It's just an else statement around the drag logic that separates it from the meta- or middle-click logic.

Comment 5

[Brian Grinstead [:bgrins]]

Comment on attachment 8639626 [details] [diff] [review]
b1187582.patch

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

San you upload a new version of this with the suggested change?  I've also got a regression test patch incoming that I'll attach to the bug

::: browser/devtools/markupview/markup-view.js
@@ +1938,5 @@
>  
>      if (isMiddleClick || isMetaClick) {
>        let link = target.dataset.link;
>        let type = target.dataset.type;
>        this.markup._inspector.followAttributeLink(type, link);

I'd rather have an early return here than the else just to limit nesting and a more accurate history on the file

Comment 6

[Brian Grinstead [:bgrins]]

Attachment: markup-link-regression-test.patch

Regression test

Comment 7

[John Pavlicek]

Attachment: b1187582.patch

Updated the patch to use a return statement instead of the else.  Do I need to do anything with the regression test patch?

Comment 8

[Brian Grinstead [:bgrins]]

(In reply to John Pavlicek from comment #7)
> Do I need to do anything with the regression test patch?

Nope, we will just check them in as two separate patches.  Here's an ongoing try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=157241d0b3e7

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/110b4a6848ab
https://hg.mozilla.org/integration/fx-team/rev/ee18f1f6fdd4

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/110b4a6848ab
https://hg.mozilla.org/mozilla-central/rev/ee18f1f6fdd4