476237 - Can't drop between two folders on Personal Toolbar anymore

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090130 Minefield/3.2a1pre] (nightly) (W2Ksp4)

Works fine.

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b3pre) Gecko/20090130 SeaMonkey/2.0a3pre] (nightly) (W2Ksp4)
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090130 SeaMonkey/2.0a3pre] (experimental/_m-c_, home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/f1493cf102b9
 +http://hg.mozilla.org/comm-central/rev/...)

Broken: either left or right folder opens.

Fwiw, droppping between two regular items still works.

Comment 1

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a5pre) Gecko/20070515 SeaMonkey/1.5a] (nightly) (W2Ksp4)
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b3pre) Gecko/20090101 SeaMonkey/2.0a3pre] (nightly) (W2Ksp4)
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b3pre) Gecko/20090126 SeaMonkey/2.0a3pre] (nightly) (W2Ksp4)

Work.

(Note that there are two drop positions between each folder or item :-/
 To be fixed here or in a followup bug...)

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b3pre) Gecko/20090127 SeaMonkey/2.0a3pre] (nightly) (W2Ksp4)
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b3pre) Gecko/20090130 SeaMonkey/2.0a3pre] (nightly) (W2Ksp4)

Fail.

Regression timeframe:
http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?startdate=2009-01-25+23%3A00&enddate=2009-01-27+01%3A00
http://hg.mozilla.org/comm-central/pushloghtml?startdate=2009-01-25+23%3A00&enddate=2009-01-27+01%3A00
My first guess would be (c-c) bug 471372.

Comment 2

[Philip Chee]

Neil, I have no idea how the BookmarksMenuDNDObserver works.

Comment 3

[[e-mail]]

Nothing to do with customise toolbar, this is fallout from bug 474807.

The problem is that there is no such computed style property as "-moz-padding-start", so we end up trying to compare a number to a NAN.

Ironically neither of our themes specify padding on the element in question.

Comment 4

[Simon Paquet [:sipaq]]

Neil is right, http://mxr.mozilla.org/comm-central/source/suite/common/bookmarks/bookmarksMenu.js#230 should not use "-moz-padding-start" but "padding-left".

Comment 5

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) Undo this part of bug 474807 (++)

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090130 SeaMonkey/2.0a3pre] (experimental/_m-c_, home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/f1493cf102b9
 +http://hg.mozilla.org/comm-central/rev/...)

Restore previous behavior.

Comment 6

[Philip Chee]

+        border = Math.min(size / 5, Math.max(border, 4));
       } else
         border = size/5;
     else
       border = size/2;

I think if the rest of the file uses |size/n| without whitespace, you should be consistent with that.

Comment 7

[jag (Peter Annema)]

I agree with the "when in Rome" rule, except in this case, since it's just those three, I would go the other way and add spaces to all three, which is the more common style for suite/.

Comment 8

[jag (Peter Annema)]

(It's not a rule so much as a guideline.)

Comment 9

[jag (Peter Annema)]

Hrm, and then there's the |clientCoordValue-coordValue| right below it... And the over-extended "&&" above. This whole file could do with style clean-up, but let's leave it to just this function right now.

Comment 10

[Serge Gautherie (:sgautherie)]

Attachment: (Av1a) Undo this part of bug 474807 (++) [Checkin: See comment 12]

Av1, with comment 7 and comment 9 suggestion(s).

Comment 11

[jag (Peter Annema)]

Comment on attachment 359998 [details] [diff] [review]
(Av1a) Undo this part of bug 474807 (++)
[Checkin: See comment 12]

I like it, but Neil had some ideas on how to simplify that code if we make some assumptions.

As a final suggestion: split |default: return BookmarksUtils.DROP_ON;| into two lines. Wait for Neil's input though before attaching a new patch.

Comment 12

[Serge Gautherie (:sgautherie)]

Comment on attachment 359998 [details] [diff] [review]
(Av1a) Undo this part of bug 474807 (++)
[Checkin: See comment 12]


http://hg.mozilla.org/comm-central/rev/8c79d2928abd
Av1a, with comment 11 suggestion(s).

Comment 13

[Serge Gautherie (:sgautherie)]

(In reply to comment #1)
> (Note that there are two drop positions between each folder or item :-/
>  To be fixed here or in a followup bug...)

I filed bug 476394.