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

NEW
Unassigned

Status

()

defect
P3
normal
2 years ago
Last month

People

(Reporter: soeren.hentzschel, Unassigned, NeedInfo)

Tracking

({regression})

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox54 unaffected, firefox55+ wontfix, firefox56 wontfix, firefox57 wontfix, firefox66 wontfix, firefox67 fix-optional, firefox68 wontfix)

Details

(Whiteboard: [reserve-photon-animation])

Attachments

(4 attachments)

Reporter

Description

2 years ago
Posted video tabs.mov
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
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.
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)
Posted video tab-thingy.mov
Yeah, still repros for me on today's Nigthtly. Attaching screencast.
Flags: needinfo?(miket)
(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)
Comment hidden (mozreview-request)
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 20

2 years ago
mozreview-review
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.
Duplicate of this bug: 1537872

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).

You need to log in before you can comment on or make changes to this bug.