941372 - Icons in the 'More Tools' section can get misplaced while being rearranged

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[mjh563]

Attachment: screenshot showing misplaced icon

Steps to Reproduce:

1. Start Firefox using a new profile.
2. Enter Customization mode.
3. If necessary, resize the window so that the 6 icons shown under "More Tools to Add to the Menu and Toolbar" are in a row of 4 followed by a row of 2.
4. The Developer icon should be the second from left in the top row. Drag it left until it's over the position of the left-most icon in that row.

Expected result:

The other icons adjust their position during the drag, but should remain in the existing rows and columns.

Actual result:

One of them half-disappears off the left of the window. See the attached screenshot.

Comment 1

[:Gijs (out until Jan 5th; he/him)]

This is because we store a negative _horizontalDistance on the dragpositionmanager for the palette. There's probably just a sign invert going on (ie it should just be abs(x) where it is now x) but I should look at this again after sleeping to be sure that the code generating that variable is correct.

We're using that value because the last item doesn't have a next sibling we can use to determine how far to move it - nor does it have a previous sibling, because when you take out the devtools item, it's the first and last item in the second row. So we use a stored value to determine how far to move it. The stored value is wrong (so we move backwards instead of forwards). Because we move backwards, the code also decides to move the item down a row. That'd normally be correct, except we shouldn't be moving backwards in the first place. :-)


Thanks again for the detailed STR! Very helpful when figuring out stuff like this.

Comment 2

[:Gijs (out until Jan 5th; he/him)]

Attachment: [Australis] ignore hidden elements for drag calculations,

Sometimes the solution is quite simple. When dragging the developer button (second item), that item becomes hidden. We then used 0 and the position of the previous item for the calculation. That, err, doesn't work very well. We should just ignore hidden items.

Comment 3

[Mike de Boer [:mikedeboer]]

Comment on attachment 8335983 [details] [diff] [review]
[Australis] ignore hidden elements for drag calculations,

Review of attachment 8335983 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm.

Comment 4

[:Gijs (out until Jan 5th; he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/078d1a3834b6

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/078d1a3834b6