Closed Bug 476237 Opened 11 years ago Closed 11 years ago

Can't drop between two folders on Personal Toolbar anymore

Categories

(SeaMonkey :: Bookmarks & History, defect)

x86
Windows 2000
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0a3

People

(Reporter: sgautherie, Assigned: sgautherie)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

[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.
Flags: blocking-seamonkey2?
[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.
Blocks: 471372
Neil, I have no idea how the BookmarksMenuDNDObserver works.
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.
Blocks: 474807
No longer blocks: 471372
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".
Target Milestone: --- → seamonkey2.0a3
[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.
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #359964 - Flags: superreview?(neil)
Attachment #359964 - Flags: review?(neil)
+        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.
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/.
(It's not a rule so much as a guideline.)
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.
Av1, with comment 7 and comment 9 suggestion(s).
Attachment #359964 - Attachment is obsolete: true
Attachment #359998 - Flags: superreview?(neil)
Attachment #359998 - Flags: review?(jag)
Attachment #359964 - Flags: superreview?(neil)
Attachment #359964 - Flags: review?(neil)
Attachment #359998 - Flags: review?(jag) → review+
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.
Attachment #359998 - Flags: superreview?(neil) → superreview+
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).
Attachment #359998 - Attachment description: (Av1a) Undo this part of bug 474807 (++) → (Av1a) Undo this part of bug 474807 (++) [Checkin: Comment 12]
(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.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: blocking-seamonkey2?
Resolution: --- → FIXED
Attachment #359998 - Attachment description: (Av1a) Undo this part of bug 474807 (++) [Checkin: Comment 12] → (Av1a) Undo this part of bug 474807 (++) [Checkin: See comment 12]
You need to log in before you can comment on or make changes to this bug.