Closed Bug 1364669 Opened 7 years ago Closed 3 years ago

Tab shape appears on an inactive tab after moving one tab from the right to the left

Categories

(Core :: DOM: Copy & Paste and Drag & Drop, defect, P3)

Product:
Core
Component:
DOM: Copy & Paste and Drag & Drop
Type:
defect

Tracking

()

Status:
RESOLVED WORKSFORME
Tracking Flags:
Tracking Status
firefox54 --- unaffected
firefox55 + wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox66 --- wontfix
firefox67 --- fix-optional
firefox68 --- wontfix

People

(Reporter: soeren.hentzschel, Unassigned)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression, Whiteboard: [reserve-photon-animation])

Attachments

(4 files)

tabs.mov
(deleted), video/quicktime
Details
tab-thingy.mov
(deleted), video/quicktime
Details
tab-thinger-windows.mov
(deleted), video/quicktime
Details
Bug 1364669 - Tried to implement mikedeboer's recommendation, still reproducing bug.
(deleted), text/x-review-board-request
Details
Attached video tabs.mov (deleted) —
As you can see in the screencast there is a visible tab shape on an inactive tab after moving on tab from the right to the left. It seems to be a regression in of the last Nightly builds, maybe from bug 1355507.
[Tracking Requested - why for this release]: recent regression in primary UI
Blocks: 1355507
status-firefox54: --- → unaffected
status-firefox55: --- → affected
tracking-firefox55: --- → ?
Component: Theme → Tabbed Browser
Summary: Tab animation regression (moving tabs) → Tab shape appears on an inactive tab after moving one tab from the right to the left
Tracking 55+ for the reason in Comment 1.
tracking-firefox55: ? → +
Dao, do you think we can get to this in the 55 timeframe?
Flags: needinfo?(dao+bmo)
STR: 1) open nightly 2) open a few tabs 3) drag right-most tab to the left a few positions Note: this only reproduces if you keep the mouse pointer hovering over the new tab position, once you move it (even just a pixel ) the new tab position the right-most tab loses hover styles.
Jared, since you worked on bug 1355507 maybe you can help here?
Flags: needinfo?(jaws)
I'm unable to reproduce this bug, even with Mike's STR from comment #4 and making sure that I don't move the mouse cursor a single pixel.
Flags: needinfo?(jaws)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6) > I'm unable to reproduce this bug, even with Mike's STR from comment #4 and > making sure that I don't move the mouse cursor a single pixel. I failed to reproduce this, too.
Hey Mike and Sören, are either of you still able to reproduce this?
Flags: needinfo?(miket)
Flags: needinfo?(cadeyrn)
Attached video tab-thingy.mov (deleted) —
Yeah, still repros for me on today's Nigthtly. Attaching screencast.
Flags: needinfo?(miket)
Attached video tab-thinger-windows.mov (deleted) —
(ni? jaws just to make sure you see this comment, since you're not on irc atm): I'm able to reproduce on Win 10 as well... but it's slightly more edge-casey. When I "drop" the tab after moving it, I have to make sure the mouse pointer is at the very top of the tab strip, so the cursor turns into a vertical resize cursor. Then
Flags: needinfo?(jaws)
(In reply to Mike Taylor [:miketaylr] (55 Regression Engineering Owner) from comment #10) > I'm able to reproduce on Win 10 as well... but it's slightly more > edge-casey. When I "drop" the tab after moving it, I have to make sure the > mouse pointer is at the very top of the tab strip, so the cursor turns into > a vertical resize cursor. This happens around the 0:18 second mark in the tab-thinger-windows.mov attachment.
Thanks! I can repro it now.
Flags: needinfo?(jaws)
Flags: needinfo?(cadeyrn)
"beforehovered" is on showing up on the wrong tab, causing the tab hovered shape to be visible. I've tried the following code at [1] thinking that something related to _hoveredTab is incorrect, but it didn't fix it, > this._finishAnimateTabMove(); > if (dropIndex !== false) { >+ draggedTab._mouseleave(); > this.tabbrowser.moveTabTo(draggedTab, dropIndex); >+ draggedTab._mouseenter(); > } Mike, I see you added the _mouseenter() method in bug 879597. What do you think might be going wrong here? [1] http://searchfox.org/mozilla-central/rev/ae94cfb36d804727dfb38f2750bfcaab4621df6e/browser/base/content/tabbrowser.xml#7064-7066
Flags: needinfo?(mdeboer)
I think you basically need to set a flag on the tabContainer that we're in dragging mode, so any hovering is ignored during that time, which'd mean that '_hoveredTab', '_beforeHoveredTab' and '_afterHoveredTab' are unset. You might want to call `if (this._hoveredTab) this._hoveredTab._mouseleave()` when a dragging action is started. Does that help?
Flags: needinfo?(mdeboer)
Set NI on Jared for comment 14 :)
Flags: needinfo?(jaws)
Thank you for the needinfo ping.
Flags: needinfo?(jaws)
Attachment #8886390 - Flags: review?(mdeboer)
Hey Pete, can you give some guidance on how important this bug is for 55? THanks.
Flags: needinfo?(pdolanjski)
Since this is primary UI and a regression, I'd really like it fixed for 55. If there is significant risk, we can re-evaluate as it is somewhat an edge-case and likely not easily noticed.
Flags: needinfo?(pdolanjski)
Comment on attachment 8886390 [details] Bug 1364669 - Tried to implement mikedeboer's recommendation, still reproducing bug. https://reviewboard.mozilla.org/r/157152/#review164248 ::: browser/base/content/tabbrowser.xml:7559 (Diff revision 1) > <field name="closing">false</field> > --> > > <method name="_mouseenter"> > <body><![CDATA[ > - if (this.hidden || this.closing) > + let tabContainer = this.parentNode; The only thing I realized when doing another review in this area of code, is that this function may get called twice for the same tab, even though that might not be intentional. So it'd make sense to me to try and guard, like: ```js if (this.hidden || this.closing || tabCotainer._blockHoverChanges || tabContainer._hoveredTab == this) return; //... continue. ``` I'd be curious to hear if that changes anything for you.
Attachment #8886390 - Flags: review?(mdeboer)
Tentatively taking this so it stays on my radar.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Iteration: --- → 56.3 - Jul 24
Flags: qe-verify+
Priority: -- → P1
QA Contact: jwilliams
Whiteboard: [reserve-photon-animation]
Iteration: 56.3 - Jul 24 → 56.4 - Aug 1
Iteration: 56.4 - Aug 1 → 57.1 - Aug 15
Flags: needinfo?(dao+bmo)
Iteration: 57.1 - Aug 15 → 57.2 - Aug 29
Iteration: 57.2 - Aug 29 → 57.3 - Sep 19
QA Contact: jwilliams → stefan.georgiev
Assignee: jaws → nobody
Status: ASSIGNED → NEW
Iteration: 57.3 - Sep 19 → ---
Priority: P1 → P4
I have reproduced this issue using Firefox 57.0b12 on Windows 8.1 x64, Mac OS X 10.13 and Ubuntu 14.04 x64.
status-firefox57: --- → affected

I am pretty certain this is an issue in our Drag & Drop / Mouse event handling.

Using the STR from comment #10, and putting a logging statement in tabbrowser.xml's _mouseenter, we get a mouseenter event after the drop and it is the exact same coordinates (clientX, clientY) as the mouseenter event prior to the dragstart.

Component: Tabbed Browser → Drag and Drop
Priority: P4 → P3
Product: Firefox → Core

Neil, could you look in to this?

Flags: needinfo?(enndeakin)

Bulk change to wontfix for 68 (P3+ carryover with needinfo).

status-firefox68: affected → wontfix
No longer blocks: 1355507
Regressed by: 1355507
Blocks: 1637238

Is this still relevant?

Flags: needinfo?(enndeakin) → needinfo?(soeren.hentzschel)

(In reply to Neil Deakin from comment #27)

Is this still relevant?

I don't think so. Since we saw two redesigns in the meantime let's close this ticket.

Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(soeren.hentzschel)
Resolution: --- → WORKSFORME
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: