Seen while running Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2b4pre) Gecko/20091116 Namoroka/3.6b4pre. This bug was identified by Lukas Blakk during an IRC conversation this AM. STR: 1. Using a clean profile, open several tabs. 2. Bookmarks | Bookmarks All Tabs 3. Give the folder a name 4. Expand the Bookmarks menu Expected: The name would show in the dropdown list. Actual: The menu shows [Folder Name]. However in Organize Bookmarks the name shows correctly. I tested using 10.4 and see this bug there, but not using windows 7.

This works correctly if the folder is created on the Bookmarks Toolbar. However, the default selection for Bookmark All Tabs is for the Bookmarks Menu.

This worked for me using the same build as Maria, using an old profile.

I was able to reproduce on 10.4 PPC. However, after a restart the [New Folder] is now "foo Folder" as I named it. Weird. Can't reproduce on Linux.

this is most likely a view update problem. i cannot reproduce on Win Vista, but i can reproduce on Snow Leopard.

"view update problem" means the change has been actually executed by backend, but frontend code (the view) did not update (thus explaining why you see it correctly on restart)

yeah, that makes sense. The Bookmark menu item for the folder is correct even in a "new window" while as original window menu remains wrong. I can also reproduce on 10.6 on a nightly build from 20091110.

nodeTitleChanged is correctly called, with correct param. We set nodeElt.label = new_title but looks like that does not work on mac native menu. Also reading nodeElt.label does not return anything while it should return "[New Folder]"

nodeElt is the correct element, using getAttribute("label") and setAttribute("label") works correctly, using the .label property does not. funny.

looks like on mac native menus the dom elements are not QI like on other platforms, Mano, can this create us issues related to bug 498130?

Mano is investigating a better "workaround" for the xbl bug behind this.

Not blocking, but would take a patch now or in a security and stability release.

reassigning

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h". In Thunderbird 3.0b, you do that as follows: Tools | Message Filters Make sure the correct account is selected. Click "New" Conditions: Body contains places-to-b-and-h Change the action to "Delete Message". Select "Manually Run" from the dropdown at the top. Click OK. Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter. Gerv

Comment on attachment 415133 [details] [diff] [review] patch >diff --git a/browser/components/places/content/menu.xml b/browser/components/places/content/menu.xml >+#ifdef XP_MACOSX >+ function applyBinding(elm) { >+ let elmURI = /^url\("(.+)"\)$/.exec( >+ window.getComputedStyle(element, "") >+ .getPropertyValue("-moz-binding")); >+ if (elmURI.length == 2) >+ document.addBinding(elm, elmURI[1]); >+ } a couple questions: https://developer.mozilla.org/en/XBL/XBL_1.0_Reference/DOM_Interfaces tells that addBinding is not implemented, but clearly it is, so i don't believe that doc a lot. But it's interesting that it's saying "The binding may not be attached yet when the call completes.", i suppose in our case this won't be a big deal since the user can't be lightning fast. afaik, addBinding can add multiple bindings to an element, is there a risk that our element ends up having the same mxnu.xml binding attached twice (once forced and once lazily through the css definition)? Or are we sure the css binding is ignored forever? I also suppose there is no way for an extension to add a different (or multiple) -moz-binding through css, that would cause our regexp to fail? i guess if we should make the regexp more "specialistic" making it look explicitely for a "url(.*menu\.xml.*)", just to be sure to match the right thing.

> "The binding may not be attached yet when the call completes." chrome: bindings will be loaded synchronously. Others won't be. As far as that goes, the getPropertyValue call there is not quite right in general but probably ok here given the restricted range of URIs you're likely to be dealing with. Though the last paragraph of comment 16 makes me wonder whether the range is as restricted as you might think....

> afaik, addBinding can add multiple bindings to an element, is there a risk that > our element ends up having the same mxnu.xml binding attached twice (once > forced and once lazily through the css definition)? Or are we sure the css > binding is ignored forever? Given our last talks, I cannot think of a case in which the binding is applied through css... actually the chevron uses it. Is there any way to check which bindings are applied to a node? > I also suppose there is no way for an extension to add a different (or > multiple) -moz-binding through css, that would cause our regexp to fail? i > guess if we should make the regexp more "specialistic" making it look > explicitely for a "url(.*menu\.xml.*)", just to be sure to match the right > thing. Will do.

Oh, and I cannot think of reason to opt-out extension's bindings.

Comment on attachment 415616 [details] [diff] [review] patch >diff --git a/browser/components/places/content/menu.xml b/browser/components/places/content/menu.xml >+#ifdef XP_MACOSX >+ // Bug 529062: i'd prefer us pointing to the xbl bug that should be fixed in future (assumed we have one), the blame can already show when this was added >+ function applyBinding(elm) { >+ // getPropertyCSSValue is semi-deprecared, but there's no >+ // alternative for parsing css_uri values reliably. >+ let elmBindingURI = window.getComputedStyle(element, "") >+ .getPropertyCSSValue("-moz-binding"); >+ if (elmBindingURI.primitiveType == CSSPrimitiveValue.CSS_URI) >+ document.addBinding(elm, elmBindingURI.getStringValue()); >+ } >+ >+ applyBinding(element) missing semicolon

IIRC, according to Boris, there's no xbl bug on that, except "implement xbl2".

"the xbl bug" would be "completely change how binding attachment works in an incompatible way". So yes, "implement the binding attachment mechanism from xbl2".

so let's implement XBL2 :) thanks for clarification, this is fine then.

Comment on attachment 415616 [details] [diff] [review] patch I think this matters for final release, so i'm asking pre-approval for 1.9.2, i know it's a bit late but due to this bug bookmarks names changes would not be reflected in Mac menu, and possibily other subtle bugs.

fixed semicolon http://hg.mozilla.org/mozilla-central/rev/cc4e262d3c5a

Comment on attachment 415616 [details] [diff] [review] patch a1922=beltzner, where are my tests, Marco?

i'm not sure what kind of test we could do on native mac menus. will try asking Mano if he has any idea to check the binding.

looks like we can sorta test this, indeed we have a test that was disabled on mac due to native menubar, but looks like i workarounded the problem with Neil's help.

This re-enables the views update test that was disabled on Mac due to native menubar. I added a title change check to ensure it is reflected in the views.

Comment on attachment 430377 [details] [diff] [review] test update r=mano

test updated on trunk: http://hg.mozilla.org/mozilla-central/rev/d70ef56fdca6

patch: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/03edf3418e14 test: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/dd2df3a6e001

Verifed fix on 1.9.2 branch: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.2) Gecko/20100316 Firefox/3.6.2 Also verified on trunk: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a4pre) Gecko/20100321 Minefield/3.7a4pre