#Cannot customize firefox toolbar code
This is the second approach, works for most users but those who have a few non-default bookmarks on toolbar.Īlex, please, do you have any ideas? We can do both options, it's just matter of how complex the code becomes if we want to really count default bookmarks on toolbar. > number of children in the toolbarFolder, and not collapsing if > 5 or something > I wonder if we could get away with just checking the doing the unproper thing will work for most users, will fail for some (few bookmarks on toolbar but not default ones) and for all third party distributions (that customized bookmarks on toolbar). doing the proper thing is expensive and involves a lot of code.Ģ. > Given those, I think this is feedback-, but I don't really have any suggestionsĪnd that's my greatest concern, there is not a perfect solution.ġ. But First I need to figure out if the approach has to be dropped. I could maybe consider that if browser.migration pref does not exist could be a new profile. The impact is probably towards half a second, can be optimized with some count as you said and we could maybe detect new profiles better and skip it. > comparison feels like something that would best be avoided > only run it once, granted, but forcing an import from disk + doing an O(N*M) > - this seems like it could have a pretty serious impact on startup time. Right, but default bookmarks did not change much in the last years, probably they did not change at all (on toolbar at least, we removed Get bookmarks Add-ons from the menu). > right results for someone who has different defaults from some previous > - we're only comparing current default bookmarks, so this won't produce the Thus I guessed "something less than a half", that is the 40%.
#Cannot customize firefox toolbar update
In our case there is 1 smart bookmark and 2 bookmarks, so it's 33% and 66%, but I'd not want to update this code for each change in default bookmarks. really that significant a portion of the default set?
> - where does 40% come from? Seems kind of low. I'm tempted to wontfix the bug and let users uncollapse their toolbar :( This will increasing the code size and I'm starting feeling bad about it. We could annotate it in the importExportService though, so on the temp import we can recognize it. It's hard to tell if a certain folder is the toolbar on a import, I'd guess there is no way today. > that it creates a subfolder at index 0, and it only checks items in that > - _getDefaultBookmarksRatio makes some assumptions about bookmarks.html (e.g. So I'm not sure why the browser part has not inherited that code, probably because as of today nobody tried to persist a common attribute like "collapsed". Indeed it does not work if you don't persist it. If you look at nsXULDocument.cpp you see it is doing this, because you can't tell offhand if a toolbar is already persisted ( ). It's needed because you can't persist an attribute of a toolbar if the toolbar itself is not in the persisted set. > - not sure about the implications of the _setPersist change, or why that's I wonder if we could get away with just checking the number of children in the toolbarFolder, and not collapsing if > 5 or something like that - ask faaborg? Given those, I think this is feedback-, but I don't really have any suggestions for a better approach. We'll only run it once, granted, but forcing an import from disk + doing an O(N*M) comparison feels like something that would best be avoided, especially considering that this code runs even for new profiles (at the very least it seems like you could avoid the comparisons entirely if the child counts are very different) this seems like it could have a pretty serious impact on startup time. we're only comparing current default bookmarks, so this won't produce the right results for someone who has different defaults from some previous version, right? where does 40% come from? Seems kind of low. that it creates a subfolder at index 0, and it only checks items in that folder), so it probably shouldn't have such a generic name, and should probably use PlacesUtils.toolbarFolderId directly rather than taking aFolderID _getDefaultBookmarksRatio makes some assumptions about bookmarks.html (e.g. not sure about the implications of the _setPersist change, or why that's necessary Sorry for the really large feedback delay :(Ī couple of concerns (in increasing order of magnitude):
0 kommentar(er)
