Thought i fixed this hm..
Oh this is the popup. Well ok then!
Created attachment 8607326 [details] [diff] [review] 1166139-marker-filter-name.patch
Comment on attachment 8607326 [details] [diff] [review] 1166139-marker-filter-name.patch Review of attachment 8607326 [details] [diff] [review]: ----------------------------------------------------------------- wow ::: browser/devtools/shared/timeline/global.js @@ +22,5 @@ > * - label: the label used in the waterfall to identify the marker. Can be a > * string or just a function that accepts the marker and returns a string, > * if you want to use a dynamic property for the main label. > + * If you use a function for a label, it *must* handle the case where > + * no marker is provided for a main label to describe all markers of this type. I think we can also add a test for this. Mock a bogus marker and somebody creating a function that doesn't handle it properly. Check if the label is the function body.
If unhandled, it will display `undefined` or throw (if accessing marker.name where there is no marker) -- the printing function will only handle in tools that implement it. Regardless, we can use better error messages here if that does happen (and if someone is adding a marker on their own, they can use a straight forward declaritive approach and most likely not use all these custom formatter functions)
Created attachment 8607696 [details] [diff] [review] 1166139-marker-filter-name.patch Added tests for when label is undefined (or returns undefined from a function) that throws an error with descriptive message on what to fix, from the perspective of someone adding a marker without familiarity of devtools
ni? jsantell for STR to verify fix
STR: Click the [filter] button (to the right of [clear]) and ensure that all entries have a color swatch as well as a readable name (not some function garbled text like: https://bugzilla.mozilla.org/attachment.cgi?id=8607305 )
Comment on attachment 8607696 [details] [diff] [review] 1166139-marker-filter-name.patch Approval Request Comment [Feature/regressing bug #]: 1167252, the new performance tool [User impact if declined]: Won't ship the performance tool [Describe test coverage new/current, TreeHerder]: There are try pushes in Bug 1167252 with all patches needing uplift [Risks and why]: Requesting uplift for the accumulated changes in the performance tool since the 40 merge date, so these changes haven't had the full 6 weeks to bake. Risks are generally contained within devtools, specifically within the performance panel. [String/UUID change made/needed]: None
Note: I had verbal confirmation for these uplifts from Sylvestre even before he's flagged them as a+. See https://bugzilla.mozilla.org/show_bug.cgi?id=1167252#c26
Comment on attachment 8607696 [details] [diff] [review] 1166139-marker-filter-name.patch Change approved to skip one train as part of the spring campaign.
Verified fixed on Aurora 40.0a2 (2015-06-04), using Ubuntu 14.04 (x64), Windows 7 (x64) and Mac OS X 10.9.5. The Filters menu is now displayed properly, with the exception of a Linux-only issue affecting the checkboxes. I've filed Bug 1172412 for that.