584411 - Moving an app tab does not show the insert pointer in the correct location when normal tabs overflow

Status: NEW

(Firefox :: Tabbed Browser, defect)

Description

[Kevin Brosnan]

Create 3 app tabs
Create enough normal tabs that we start scrolling normal tabs
Try and drag the any of the tabs to behind the last app tab

Pointer is shown either before the last app tab or behind the left scroll button.
Pointer should show behind the last app tab as a valid drop location.

Dropping an app tab behind the last app tab does place it behind the last app tab despite what the pointer shows.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Kevin, which build are you using? IMO this is a dupe of bug 579472.

Comment 2

[Kevin Brosnan]

Attachment: Screenshot

It is an edge case that was not fixed by bug 579472.

Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3pre) Gecko/20100804 Minefield/4.0b3pre

Comment 3

[Sindre Dammann]

Attachment: Patch

Couldn't figure out how to make a test for this. :(

Comment 4

[Sindre Dammann]

Attachment: Patch v2

It somehow uploaded an old version of the patch.

Comment 5

[Sindre Dammann]

Attachment: Patch v3

Turns out I saved the new patch in a different directory, and therefore managed to upload the old patch twice.

Embarrassing.

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 470200 [details] [diff] [review]
Patch v3

>+          let tab = this._getDragTargetTab(event);

This is unused.

>+          if (newIndex < this.tabbrowser._numPinnedTabs) {
>             if (ltr)
>               newMargin = tabRect.left - rect.left;
>             else
>               newMargin = rect.right - tabRect.right;
>+          } else {
>+            // When dragging over a non-apptab position, make sure the arrow
>+            // doesn't appear in front of the scroll button.
>+            if (ltr)
>+              newMargin = Math.max(tabRect.left - rect.left, minMargin);
>+            else
>+              newMargin = Math.max(rect.right - tabRect.right, minMargin);
>           }

This seems identical to:

if (ltr)
  newMargin = tabRect.left - rect.left;
else
  newMargin = rect.right - tabRect.right;

if (newIndex >= this.tabbrowser._numPinnedTabs)
  newMargin = Math.max(newMargin, minMargin);

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 470200 [details] [diff] [review]
Patch v3

per comment 7