1250806 - 5 most recently added bookmarks are not available in the Menu bar - Bookmarks on Mac OS X

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Mihai Boldan, Desktop QA [:mboldan]]

[Affected versions]:
- Firefox 47.0a1 (2016-02-23)

[Affected platforms]:
- Mac OS X 10.9.5, Mac OS X 10.11

[Steps to reproduce]:
1. Launch Firefox.
2. Bookmark a few pages.
3. Go to Menu Bar -> Bookmarks and observe the Recently Bookmarked section.

[Expected result]:
- The last 5 bookmarked pages are displayed in the Recently Bookmarked section.

[Actual result]:
- Nothing is displayed in the Recently Bookmarked section.

[Regression range]:
- I will verify if this is a regression ASAP.

[Additional notes]:
- This issue is not reproducible on Windows or on Ubuntu platforms.

Comment 1

[Dão Gottwald [:dao]]

Is this with a new profile? If this is indeed plain broken, I'm a little surprised that nobody else had reported this yet.

Comment 2

[Mihai Boldan, Desktop QA [:mboldan]]

The tests were performed with a clean profile. Only the Recently Bookmark title is displayed in the '5 most recently added bookmarks' section.

Comment 3

[Mihai Boldan, Desktop QA [:mboldan]]

This is not a regression since the issue is reproducible on Firefox 47.0a1 (2016-02-14), build where 'Recently Bookmarked' section was implemented.

Comment 4

[Dão Gottwald [:dao]]

Marco, any idea what's going on here? We're adding the menuitems in the popupshowing event handler. AFAIK this should just work™ on OS X.

Comment 5

[Dão Gottwald [:dao]]

Oh wait, we're actually adding the menuitems asynchronously using asyncExecuteLegacyQueries. I guess this ends up being too late. I thought the native menubar would support live updates, but maybe not. How do we usually deal with this in places menus?

Comment 6

[Marco Bonardo [:mak]]

no the native menubar only updates content in onpopupshowing, unfortunately.
We deal with it by populating before it shows... That doesn't work very well with async stuff, indeed we still have an open bug (that atm I cannot find) to add live update support to the native menus...

Comment 7

[Marco Bonardo [:mak]]

I think we only update the menu contents here
http://mxr.mozilla.org/mozilla-central/source/widget/cocoa/nsMenuX.mm#581
and when there is new content, we set a flag that tell to rebuild the menu at the next opening.

Comment 8

[Dão Gottwald [:dao]]

(In reply to Marco Bonardo [::mak] from comment #6)
> no the native menubar only updates content in onpopupshowing, unfortunately.
> We deal with it by populating before it shows...

How exactly? Should I use something other than asyncExecuteLegacyQueries or listen to some other popup event?

Comment 9

[Marco Bonardo [:mak]]

The queries populating the menubar ATM are synchronous (just PlacesUtils.history.executeQuery), that is not very nice for jank, but probably the only solution for the native menubar, unless someone fixes live updating.

Comment 10

[Dão Gottwald [:dao]]

Attachment: patch

Comment 11

[Marco Bonardo [:mak]]

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

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

I didn't check on Mac, but it should work off-hand.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/f7149b0bcf74

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f7149b0bcf74

Comment 14

[Mihai Boldan, Desktop QA [:mboldan]]

The 5 most recently added bookmarks are correctly displayed on Firefox 47.0a1 (2016-03-07)and on Firefox 48.0a1 (2016-03-08).
The tests were performed on Mac OS X 10.9.5 and on Mac OS x 10.11.1.
I am marking this issue Verified-Fixed.