Dragging tabs is completely broken

VERIFIED FIXED in Firefox 57

Status

()

Firefox
Tabbed Browser
--
major
VERIFIED FIXED
7 months ago
24 days ago

People

(Reporter: Oriol, Assigned: dao)

Tracking

(Blocks: 1 bug, {regression})

57 Branch
Firefox 57
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox57 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

7 months ago
1. Open lots of tabs (e.g. use Ctrl+T)
2. Mousedown a tab, drag it to some side

Expected: The tab is reordered to where the mouse is.
Result: It moves to another place.

I have 500 opened tabs, with this I lose track of them and Nightly is completely unusable. This needs a quick fix or backout but 1387084.

Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fe13b9b9e3cea1a8d4231ae44ccdec5261c6711f&tochange=11d86d5c215907a233558afefd1eb0d20b50b7d5
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

7 months ago
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)

Comment 4

7 months ago
mozreview-review
Comment on attachment 8894273 [details]
Bug 1387861 - Use _scrollbox.scrollLeft instead of scrollPosition which doesn't exist anymore, and update remaining _handleTabSelect call sites for signature change.

https://reviewboard.mozilla.org/r/165360/#review170598

r=me to unbreak stuff, but:

1) please file a bug to ensure we have better automated test coverage of this. This regression shouldn't have survived automated tests. We have automated test coverage of dnd on about:newtab (or at least, we used to before activity stream), those might be a useful template to work from as it might require synthesizing native events.
2) please file a bug to update/remove the remaining mentions of `scrollPosition` (devtools, test_mousescroll.xul). They don't look to me like they're actually still current, but we should tidy up the loose ends to be sure.
Attachment #8894273 - Flags: review?(gijskruitbosch+bugs) → review+

Updated

7 months ago
Component: XUL Widgets → Tabbed Browser
Product: Toolkit → Firefox
[Tracking Requested - why for this release]:
status-firefox55: --- → unaffected
status-firefox56: --- → unaffected
status-firefox57: --- → affected
status-firefox-esr52: --- → unaffected
tracking-firefox57: --- → ?
Keywords: regression

Comment 6

7 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f2df8633a4ff
Use _scrollbox.scrollLeft instead of scrollPosition which doesn't exist anymore, and update remaining _handleTabSelect call sites for signature change. r=Gijs

Comment 11

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f2df8633a4ff
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57

Updated

7 months ago
Duplicate of this bug: 1387718

Updated

7 months ago
Duplicate of this bug: 1388042
I'm confirming that issue is FIXED, starting since Mozilla Firefox Nightly 57.0a1 (2017-08-07), so I'm marking this bug as VERIFIED.
Severity: normal → major
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
OS: Unspecified → All
QA Contact: Virtual
Hardware: Unspecified → All
Version: unspecified → 57 Branch

Updated

7 months ago
Duplicate of this bug: 1388324
(In reply to :Gijs from comment #4)
> 1) please file a bug to ensure we have better automated test coverage of
> this. This regression shouldn't have survived automated tests. We have
> automated test coverage of dnd on about:newtab (or at least, we used to
> before activity stream), those might be a useful template to work from as it
> might require synthesizing native events.
> 2) please file a bug to update/remove the remaining mentions of
> `scrollPosition` (devtools, test_mousescroll.xul). They don't look to me
> like they're actually still current, but we should tidy up the loose ends to
> be sure.

Dao, can you please take care of those two review comments from Gijs? Thanks.
Flags: needinfo?(dao+bmo)

Comment 17

7 months ago
Since this is fixed and verified, no need to track it for 57.
tracking-firefox57: ? → ---
(Assignee)

Updated

24 days ago
Blocks: 1434229
(Assignee)

Updated

24 days ago
Flags: needinfo?(dao+bmo)
You need to log in before you can comment on or make changes to this bug.