Closed Bug 1187582 Opened 4 years ago Closed 4 years ago

Opening a link in markup view attributes triggers drag action

Categories

(DevTools :: Inspector, defect)

x86_64
Windows 8.1
defect
Not set

Tracking

(firefox42 fixed)

VERIFIED FIXED
Firefox 42
Tracking Status
firefox42 --- fixed

People

(Reporter: sebo, Assigned: sr71pav)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

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
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?
Flags: needinfo?(sr71pav)
Keywords: regression
I can look into it, no problem.
Flags: needinfo?(sr71pav)
Thanks!
Assignee: nobody → sr71pav
Status: NEW → ASSIGNED
Attached patch b1187582.patch (obsolete) — Splinter Review
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.
Attachment #8639626 - Flags: review?(bgrinstead)
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
Attachment #8639626 - Flags: review?(bgrinstead)
Regression test
Attachment #8640145 - Flags: review+
Attached patch b1187582.patchSplinter Review
Updated the patch to use a return statement instead of the else.  Do I need to do anything with the regression test patch?
Attachment #8639626 - Attachment is obsolete: true
Attachment #8640215 - Flags: review?(bgrinstead)
Attachment #8640215 - Flags: review?(bgrinstead) → review+
(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
https://hg.mozilla.org/mozilla-central/rev/110b4a6848ab
https://hg.mozilla.org/mozilla-central/rev/ee18f1f6fdd4
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.