Closed Bug 1235129 Opened 8 years ago Closed 8 years ago

Drag-n-drop url from location bar to a <tab> opens new tab instead of loading url in that tab

Categories

(Firefox :: Tabbed Browser, defect)

44 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 47
Tracking Status
firefox43 --- unaffected
firefox44 + wontfix
firefox45 + verified
firefox46 + verified
firefox47 --- verified

People

(Reporter: arni2033, Assigned: enndeakin)

References

Details

(Keywords: regression)

Attachments

(1 file)

>>>   My Info:   Win7_64, Nightly 46, 32bit, ID 20151223030323
STR:
1. Open http://example.com/ in a new tab
2. Open https://yandex.ru/  in a new tab
3. Switch to tab from Step 1
4. Select all text in urlbar
5. Drag-n-drop it to the tab from Step 2

Result:       
 Url http://example.com/ opened in a new tab

Expectations: 
 Url http://example.com/ should be loaded in the tab from Step 2


Regression range (it's not very precise, but I'm sure bug 1207594 regressed this):
>   pushlog_url: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9a8f2342fb3116d23989087e026448d38a3768c5&tochange=5430b2dba98b2a39ccdfd3700131f780a27be17c
This seems like a basic scenario, tracked for 44.
Neil, based on the regression window in comment 1, would you be able to help investigate and perhaps provide a fix? I think this is a very basic and oft-used scenario that we regressed in 44 and should try fixing it before we release.
Flags: needinfo?(enndeakin)
Just like Ritu said!
Benjamin, would you consider this a release blocker? I definitely think this is an oft-out scenario that we have regressed in Fx44. Would you be able to help find an owner and possibly a patch? I think this one meets the beta44 uplift criteria "critical (recent) regressions". Thanks!
Flags: needinfo?(benjamin)
Neil, please take a look.

I have no opinion about whether this is a release blocker. The new behavior seems strictly better than the old one, but I'm neither an owner nor even very knowledgeable about how people use drag/drop like this.
Assignee: nobody → enndeakin
Component: Drag and Drop → Tabbed Browser
Flags: needinfo?(benjamin)
Product: Core → Firefox
IMHO, the current behaviour should be restored, because the user gets 2 choices:
#1 opening a new tab by dropping the URL in the tab bar (or between 2 tabs, it's the same)
#2 replacing an existing tab by dropping the URL in the tab

With this bug, the user losts one possibility (#2).
OK, so the tabbrowser is kind of abusing the copy/move semantics of drag and drop here.

We are dragging a link or plaintext so the 'copy' drag action is correct here. The 'move' drag action would also be ok but the urlbar doesn't support it.

The tabbrowser interprets the 'copy' drag action as 'make a new tab'. That would be correct if the source were another tab, but here the source is text in the url field and we do want to copy the text from there not move it.

I think we probably want to just replace the tab content when dropping text/links on an existing tab and require dropping them between tabs or in the empty area to add a new tab. This is what Chrome and Safari do.

Maybe Dao would know some history of whether this would miss some usecase.
Flags: needinfo?(enndeakin) → needinfo?(dao)
Attached patch droptabnocopySplinter Review
This patch implements what's described above.
Flags: needinfo?(dao)
Attachment #8713206 - Flags: review?(dao)
(In reply to arni2033 from comment #0)
> STR:
> 1. Open http://example.com/ in a new tab
> 2. Open https://yandex.ru/  in a new tab
> 3. Switch to tab from Step 1
> 4. Select all text in urlbar
> 5. Drag-n-drop it to the tab from Step 2
> 
> Result:       
>  Url http://example.com/ opened in a new tab
> 
> Expectations: 
>  Url http://example.com/ should be loaded in the tab from Step 2

I can't seem to reproduce this. I get the expected result when following your steps. Am I missing something?
(In reply to Dão Gottwald [:dao] from comment #10)
> I get the expected result when following your steps. Am I missing something?
In Step 5, "the tab" means the <tab> element in tabs toolbar, not the web page in tab from Step 2
screencast: https://dl.dropboxusercontent.com/s/xutzba7r8eurgli/bug%201235129%20comment%2011.webm
(In reply to arni2033 from comment #11)
> (In reply to Dão Gottwald [:dao] from comment #10)
> > I get the expected result when following your steps. Am I missing something?
> In Step 5, "the tab" means the <tab> element in tabs toolbar, not the web
> page in tab from Step 2

Yes, I understood that, still can't reproduce. Is this a platform-specific bug? I'm on Linux.
Looks like Linux doesn't have this issue as it drops with 'link' here. Does reproduce on Windows and Mac though.
Attachment #8713206 - Flags: review?(dao) → review+
Keywords: checkin-needed
Neil is on PTO for two weeks. I'm not sure this fits into beta uplift category in any case, but if it does I'll need to find somebody else to do risk assessment and uplifts.  Ritu, thoughts?
Flags: needinfo?(rkothari)
Forwarding the NI to Sylvestre since he is driving Fx45 release.

My thoughts are that we should consider taking it. I stand by my stance that this is a basic scenario that we have regressed. The STR of dragging dropping a URL from location bar into an existing tab worked before and was broken in 44. I wanted to take a fix back then but none was available. For Fx45, if we can get SV/bug opener to validate the fix, it's worth uplifting the patch (one-liner).
Flags: needinfo?(rkothari) → needinfo?(sledru)
This patch is required to fix another scenario (below).
Summary:  "There's no indication that url will be opened in a new tab"

STR_2:
1. Open new window, open 2 new tab in that window
2. Open   data:text/html,<a href="https://ya.ru">Link</a>   in a new tab in that window
3. Hover mouse over the "Link" on that page, hold left mouse button, move mouse pointer
   to the center of any <tab> in that window.
4. Hold Ctrl key.
5. Release left mouse button.

AR:
 After Step 4 there's no drop indicator (black arrow between tabs)
 After Step 5 https://ya.ru opens in a new tab, near the tab from Step 3.

ER: Either X or Y
 [X] - open new tab
   After Step 4 drop indicator should be shown (to indicate that url will be opened in a new tab)
   After Step 5 https://ya.ru should open in a new tab, near the tab from Step 3.
 [Y] - load url in tab
   After Step 4 there should be no drop indicator
   After Step 5 https://ya.ru should load in the tab from Step 3.
https://hg.mozilla.org/mozilla-central/rev/4d32d1f87b74
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Gijs, as you know everything about Firefox, do you have an opinion on this? (should we take the patch in 45 or not) Thanks!
Flags: needinfo?(sledru) → needinfo?(gijskruitbosch+bugs)
Comment on attachment 8713206 [details] [diff] [review]
droptabnocopy

Approval Request Comment
[Feature/regressing bug #]: bug 1207594
[User impact if declined]: can't drop urls on tabs anymore to load them in that tab unless you're on linux
[Describe test coverage new/current, TreeHerder]: probably not, both because dnd is "special" for testing, and because this regressed without tests picking it up.
[Risks and why]: one-line change, so the only risk is that that one line could be wrong... The worst that could happen is that we will now open tabs in an existing tab when for some reason the user expected a new tab. I don't see why/how that would ever be the case, though - if you drop a link on a tab, presumably you intend to actually use that tab for the link...
[String/UUID change made/needed]: no.
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8713206 - Flags: approval-mozilla-beta?
Attachment #8713206 - Flags: approval-mozilla-aurora?
arni2033, could you please verify this issue is fixed as expected on Nightly 2/24 build? Thanks!
Flags: needinfo?(arni2033)
See Also: → 1250915
This bug is fixed on:   Win7_64, Nightly 47, 32bit, ID 20160225030209

Notes:
1) Comment 17 is fixed too:  expectation (Y) is fulfilled
2) Comment 20's "User impact if declined" description is not completely accurate, because I couldn't
   load dragging link in existing tab _only_ if I was dragging url from location bar OR
   was dragging a normal link while holding Ctrl (comment 17). But it's still a good description.
Status: RESOLVED → VERIFIED
Flags: needinfo?(arni2033)
Comment on attachment 8713206 [details] [diff] [review]
droptabnocopy

This has been verified on m-c and sounds pretty low risk. 
Let's take it in aurora and beta.
Attachment #8713206 - Flags: approval-mozilla-beta?
Attachment #8713206 - Flags: approval-mozilla-beta+
Attachment #8713206 - Flags: approval-mozilla-aurora?
Attachment #8713206 - Flags: approval-mozilla-aurora+
Verified fixed FX 45 RC, 46.0a2 (2016-03-01) Win 7.
You need to log in before you can comment on or make changes to this bug.