457587 - [Mac] Duplicates appear in Smart Bookmarks lists

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P1)

Description

[Chris Blore]

In the latest trunk builds, viewing Smart Bookmarks lists through the Bookmarks menu (not the "Most Visited" bookmark on the toolbar) results in a duplicate copy of the bookmarks in the list being created. This happens from the second time you open the menu in a session, adding a copy of each page/tag to the list each time you view the smart bookmark until you close Firefox. 

Deleting the smart bookmarks and resetting the preference in about:config to repopulate the query has no effect towards resolving the problem.

I am currently running Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080928020418 Minefield/3.1b1pre

Comment 1

[Dietrich Ayala (:dietrich)]

Easily reproduce-able. Each time the menu is opened, the contents of the query are appended. I got beachballing as well.

-> blocker

Comment 2

[Marco Bonardo [:mak]]

any idea on the regression range?

Comment 3

[Marco Bonardo [:mak]]

i can't reproduce on windows at all

do you have any bookmarks related extension (like "add bookmark here 2")?

Comment 4

[Chris Blore]

No, the only extensions I have that are enabled are Adblock Plus 0.7.5.5 and Ubiquity 0.1.2rc1

This is reproducable every time for me on OS X, although I haven't had time to track down the regression range.

Comment 5

[Marco Bonardo [:mak]]

i'm going to test this on osx ppc and see if i can reproduce. i have an idea about this, could be due to "empty" menuitem, and i suppose could be due to bug 418671 where i cleaned up the maker code, so please, if you have time to get a reg range, check first if that's between 2008-09-19 and 2008-09-20 or near that

Comment 6

[Marco Bonardo [:mak]]

ok, i can reproduce on mac, not on windows

Comment 7

[Marco Bonardo [:mak]]

so, aPopup._startMarker and aPopup._endMarker are undefined on Mac, so the cleanup function cannot remove old items.

This only happens on Mac, on Windows and Linux the markers are correctly initialized.

Comment 8

[Marco Bonardo [:mak]]

Mano, it feels like the binding is not applied to submenus with placespopup attribute, markers are undefined and not initialized. I could workaround this setting markers to -1 in this case only for Mac, but do you know what is the cause of this? i suppose is somewhat due to native osx menu.

Comment 9

[Marco Bonardo [:mak]]

Attachment: patch

the binding is not attached when we call _rebuild on the popup (onpopupshowing capturing phase), we could inizialize the markers on popup creation to ensure they are not undefined. But if you have a better idea on how to attack the problem, that would be better than workarounding it.

Comment 10

[Marco Bonardo [:mak]]

Comment on attachment 343479 [details] [diff] [review]
patch

oops i misread the ifndef

Comment 11

[Marco Bonardo [:mak]]

Attachment: patch

Comment 12

[Asaf Romano (gone)]

Comment on attachment 343480 [details] [diff] [review]
patch

File a bug on the native menus issue, r=mano.

Comment 13

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/31bc6d255e77

Comment 14

[Marco Bonardo [:mak]]

filed toolkit Bug 460542 for native menus

Comment 15

[Tracy Walker [:tracy]]

verified: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.1b2pre) Gecko/20081112 Minefield/3.1b2pre