See https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/DarkTheme-Inspector-Active@2x.png and https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/DarkTheme-Console-AllToggled@2x.png for examples. They are vertically shorter, and have a flatter design.

I think bug 929127 made the toolbars flat (and fixed partially this bug).

(In reply to ntim007 from comment #1) > I think bug 929127 made the toolbars flat (and fixed partially this bug). Bug 929127 was dealing with the sidebar tabs - this is dealing with the whole toolbar row. There aren't going to need to be a ton of changes here, but there will be changes.

(In reply to Brian Grinstead [:bgrins] from comment #2) > (In reply to ntim007 from comment #1) > > I think bug 929127 made the toolbars flat (and fixed partially this bug). > > Bug 929127 was dealing with the sidebar tabs - this is dealing with the > whole toolbar row. There aren't going to need to be a ton of changes here, > but there will be changes. The code from the patch included the flattened toolbar style ( attachment 8335477 [details] [diff] [review] ). >- background-image: url(background-noise-toolbar.png), linear-gradient(#3e4750, #3e4750); >+ background-color: #343c45; > border-bottom: 1px solid #060a0d; >- box-shadow: 0 1px 0 hsla(204,45%,98%,.05) inset, -1px 0 0 hsla(204,45%,98%,.05) inset, 0 -1px 0 hsla(204,45%,98%,.05) inset;

Maybe this bug could just be for the toolbar buttons and the code mirror find/go to line input.

(In reply to ntim007 from comment #4) > Maybe this bug could just be for the toolbar buttons and the code mirror > find/go to line input. The key change here is to shrink the toolbar height by a few px to match the designs, and making sure this is consistently applied across all panels. Once Bug 941673 lands, I will begin working on this. Not sure exactly what you mean about the CodeMirror find / go to line, but any CodeMirror-specific design changes should be opened as a separate bug.

Shrinks vertical height on toolbar. Also updates button design to be more flat, but not fully matching the designs in Comment 1. There are going to need to be functionality changes to get the console toolbar to work in this manner.

Can we have some screenshots please ? :)

(In reply to Girish Sharma [:Optimizer] from comment #7) > Can we have some screenshots please ? :) Sure, I've uploaded a couple. Basically, for the buttons, I got rid of the background gradients and box shadows and used the middle-ish color from each gradient instead. This isn't that close to the final comps, but I think it is a step towards them - I'm not sure if it will stick or if we will just want to go with the lower padding for this bug.

Comment on attachment 8349000 [details] [diff] [review] theme-toolbar.patch Darrin, when this build is complete can you download an executable from the build directory and give some feedback? There are also a couple of screenshots on this bug if you want to take a look at these, but the button states can be hard to get a good feel for until you click around / hover etc. Here is the build: https://tbpl.mozilla.org/?tree=Try&rev=d02343b1032a

Can you make it so the toolbars have no padding and inner items no margin?

(In reply to Paul Rouget [:paul] from comment #12) > Can you make it so the toolbars have no padding and inner items no margin? If you look at the designs, I believe that there is a bit of vertical padding on the toolbar (https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/DarkTheme-Console-AllToggled@2x.png). See above and below the text input and buttons. I have it set to 1px right now. What do you mean exactly by inner items having no margin?

(In reply to Brian Grinstead [:bgrins] from comment #13) > (In reply to Paul Rouget [:paul] from comment #12) > > Can you make it so the toolbars have no padding and inner items no margin? > > If you look at the designs, I believe that there is a bit of vertical > padding on the toolbar > (https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/ > DarkTheme-Console-AllToggled@2x.png). See above and below the text input > and buttons. I have it set to 1px right now. What do you mean exactly by > inner items having no margin? I'll formulate that in a different way: can you make it so that the tool toolbar is 22px height, like in the mockups?

Darrin, this build will be available at: https://tbpl.mozilla.org/?tree=Try&rev=249692d5003e, or you can have a look at any of the 'slimmer-*' images attached to this bug for a preview of what it looks like. Basically, I lowered the amount of height on the toolbars, and made it apply more consistently across a few of the panels. (Comment 10 and Comment 11 are still relevant with this patch).

(In reply to Brian Grinstead [:bgrins] from comment #17) > Created attachment 8349663 [details] > theme-slimmer-inspector.png I must say that until the top and bottom margins from the buttons are not removed, such a small height looks awkward.

Optimizer has also pointed out that the image icons are not square anymore (wider than they are tall). We could always tweak the existing patch (put a couple more px padding back into the buttons to make it less cramped and make them square again). Or we could wait until the buttons / breadcrumbs are reskinned before proceeding with this. For the button designs, we will need to a mockup with designs for different states like hover, active, checked, active / checked, etc. There is also an option of using the same styles that we have for the command buttons in the tab bar for icon buttons (as opposed to what is there for the console in https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/DarkTheme-Console-AllToggled@2x.png)

(In reply to Brian Grinstead [:bgrins] from comment #20) > Optimizer has also pointed out that the image icons are not square anymore > (wider than they are tall). We could always tweak the existing patch (put a To be more clear, What I meant is that the buttons with icons like the debugger stepping buttons (as compared to text buttons in console toolbar) are no longer square. If we go ahead and remove the padding from the toolbar, such that the top bottom borders of the button touches the top bottom border of the toolbar, that they will be square again. > couple more px padding back into the buttons to make it less cramped and > make them square again). > > Or we could wait until the buttons / breadcrumbs are reskinned before > proceeding with this. For the button designs, we will need to a mockup with > designs for different states like hover, active, checked, active / checked, > etc. There is also an option of using the same styles that we have for the > command buttons in the tab bar for icon buttons (as opposed to what is there > for the console in > https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/ > DarkTheme-Console-AllToggled@2x.png) Yes, I think I saw some mockup, (previously made) which had the same styles as the tabs toolbar buttons on the right, applied to all of the buttons. But maybe I am mistaken.

(In reply to Brian Grinstead [:bgrins] from comment #20) > Or we could wait until the buttons / breadcrumbs are reskinned before > proceeding with this. Yes, I think this is the way to go.

> Or we could wait until the buttons / breadcrumbs are reskinned before > proceeding with this. I agree with Paul, I think we need the updated buttons and breadcrumbs for such a slim toolbar to work. What do you need from me to make this work?

(In reply to Darrin Henein [:darrin] from comment #23) > > Or we could wait until the buttons / breadcrumbs are reskinned before > > proceeding with this. > > I agree with Paul, I think we need the updated buttons and breadcrumbs for > such a slim toolbar to work. What do you need from me to make this work? If I remember correctly, we may need new icons for the breadcrumb separators. The breadcrumb design is in Bug 936421. I will open a new bug for the button designs - for this if you can come up with a design showing all the various button states (on both a text button and an icon button) that should be enough to get started.

The slim toolbars look horrible with the current buttons, as mentioned in previous comments, we should probably wait the new toolbar buttons and the breadcrumbs first.

Rebased and simpler version of the patch. Will still need to wait until new buttons land for this to look right

Some ideas on how buttons and various controls can fit inside the thinner toolbars.

Consistently applies height and paddings across toolbars and sidebar tabs

Styles toolbar buttons to be borderless and get close to looking like Attachment 8385335 [details]. Still some questions that have come up regarding styling text only buttons with borders, text only buttons next to icon only buttons.

Binaries of WIP patches: https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bgrinstead@mozilla.com-eea7b2165c0b/

I tried the patches, and it looks quite bad in my opinion, it'd be nice if it actually matches shorlander's mockup.

Another updated patch - this time covering some of the odd cases with text buttons that float in the middle of a panel (such as 'edit and resend' on net panel), and also makes the buttons on the right of the debugger act like the others with hover and opened styling. I'm not happy with the colors for the different button states in the dark theme, but I think it is ready for ui review anyway. Pushed to try for a build: https://tbpl.mozilla.org/?tree=Try&rev=f32c0bc071e7.

Here's the styling I'm suggesting : http://jsfiddle.net/ntim/bp7uV/ Based off both shorlander's and dhenein's mockups.

Comment on attachment 8431139 [details] [diff] [review] thin-toolbar-part1.patch Review of attachment 8431139 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/shared/devtools/toolbars.inc.css @@ +355,4 @@ > -moz-padding-start: 6px; > margin: 0; > min-width: 78px; > + /* match height to devtools-toolbar - 1px for border */ While I'm much happier now because all these calculations were removed, I wonder if we could go one step further and use box-sizing: border-box where appropriate, to avoid stuff like "1px for border". Not a review comment, just sharing my thoughts. ::: browser/themes/shared/devtools/widgets.inc.css @@ +146,5 @@ > #breadcrumb-separator-before, > #breadcrumb-separator-after, > #breadcrumb-separator-normal { > width: 12px; > + height: 24px; I would also love to see values like "24px" as CSS variables. Definitely followup material.

(In reply to Victor Porof [:vporof][:vp] from comment #37) > Comment on attachment 8431139 [details] [diff] [review] > thin-toolbar-part1.patch > > Review of attachment 8431139 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: browser/themes/shared/devtools/toolbars.inc.css > @@ +355,4 @@ > > -moz-padding-start: 6px; > > margin: 0; > > min-width: 78px; > > + /* match height to devtools-toolbar - 1px for border */ > > While I'm much happier now because all these calculations were removed, I > wonder if we could go one step further and use box-sizing: border-box where > appropriate, to avoid stuff like "1px for border". Hmm, looking at that line in particular it seems the comment is inaccurate or that the value should be 23px. > ::: browser/themes/shared/devtools/widgets.inc.css > @@ +146,5 @@ > > #breadcrumb-separator-before, > > #breadcrumb-separator-after, > > #breadcrumb-separator-normal { > > width: 12px; > > + height: 24px; > > I would also love to see values like "24px" as CSS variables. Definitely > followup material. Yes! This would be awesome for a theme to be able to customize toolbar heights

I think the code is actually relatively close to how it will land, even with any UI changes, so I'm going to start trying to get this reviewed. Here is a screencast of it in action: https://www.youtube.com/watch?v=B_eB2RdMtBI

(In reply to Brian Grinstead [:bgrins] from comment #39) > I think the code is actually relatively close to how it will land, even with > any UI changes, so I'm going to start trying to get this reviewed. > > Here is a screencast of it in action: > https://www.youtube.com/watch?v=B_eB2RdMtBI It'd be nice if you could experiment with expanding separators between button groups. Kinda like the bookmarks menu button in the nav bar, or the menu panel footer.

This uses border-box sizing on devtools-toolbar to get rid of any ambiguity with the border included in the height. With just this patch applied, the buttons will look weird, but the height between toolbars, sidebar tabs, and network panel headers should all be consistent (and much smaller than it is currently).

UI is still up for discussion, but here are the code changes that have gotten it to the state as seen in the video in Comment 39. Darrin, binaries can be grabbed from https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bgrinstead@mozilla.com-5001db24a589/try-macosx64/ if you'd like to have a look

(In reply to Brian Grinstead [:bgrins] from comment #42) > Created attachment 8431563 [details] [diff] [review] > thin-toolbar-part2.patch > > UI is still up for discussion, but here are the code changes that have > gotten it to the state as seen in the video in Comment 39. > In particular, I'm not happy with how text only buttons look. With the left/right borders and no top/bottom they kind of look like tabs (see style editor), but if there are no borders they absolutely do not look like buttons and would be very hard to discover. I wonder if we should handle text buttons differently than icon buttons - adding all 4 borders and a px or 2 of vertical padding.

Rebased and trying something a bit different for text buttons. I've also refactored out the big 'reload' button that is used in Canvas, Shader, and Web Audio panels into a [big] button that includes min-height plus some additional margins on the right. Here is a screencast walking through the UI: https://www.youtube.com/watch?v=2GRzK9UzlkM. Of course, I'd like UI feedback but again I think the code is pretty close to how it will be regardless of changes so it should be fine to review.

Small mockup I created : http://jsfiddle.net/ntim/bp7uV/ What's great is that it brings out the checked state well.

Rebased and updated UI after working with Darrin on it. Victor, I also added a separator in the debugger between the prettify button and toggle breakpoints as discussed to more clearly show that grouping.

Just tried the patches. I think the buttons are a bit too faint. Can you try to add a border around the buttons that have a background ? Maybe the splitter color but with 0.2 opacity and a 2px border-radius ? Also, I noticed that the debugger pretty print button has a background, and doesn't have a checked state (I don't see the separator between the toggle breakpoints and prettify buttons btw).

I also noticed that the "Reload" buttons in the center don't have a border, which make them look weird. Those buttons also have a focus state (a border) which is quite strange.

> Also, I noticed that the debugger pretty print button has a background, and > doesn't have a checked state (I don't see the separator between the toggle > breakpoints and prettify buttons btw). > I also noticed that the "Reload" buttons in the center don't have a border, > which make them look weird. > Those buttons also have a focus state (a border) which is quite strange. Hmm, these must be issues on Windows. I'm not seeing these problems locally. Could definitely be something lost in the refactoring - I've pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=0bcc21cb2a15

Comment on attachment 8437023 [details] [diff] [review] thin-toolbar-part2.patch Review of attachment 8437023 [details] [diff] [review]: ----------------------------------------------------------------- Should address some of these observations, but otherwise this looks really good! 1. Certain scope headers in the variables view don't have a bottom border [0]. 2. Text and arrows don't appear to be vertically aligned anymore [0] (see "Function scope"). 3. The "Sources", "Call Stack", "Variables" etc. toolbar buttons are disproportionately tall vs. the every other toolbar and toolbarbutton. 4. In the network monitor, the table header is a few pixels taller than the tabs in the inspection pane [1]. 5. In the webconsole, the logs toolbar is a few pixels taller than the object inspector search toolbar [2]. 6. The autocompletion popup is the only remaining UI element with rounded borders [3]. Followup material, but I really think we ought to remove the border radius there. ::: browser/devtools/canvasdebugger/canvasdebugger.xul @@ +48,5 @@ > pack="center" > flex="1"> > <button id="reload-notice-button" > class="devtools-toolbarbutton" > + big="true" IMHO this is a much too generic attribute name, and doesn't really tell me why it's needed. Maybe another more descriptive class would be more appropriate? ::: browser/devtools/debugger/debugger-view.js @@ +537,1 @@ > button.setAttribute("tooltiptext", this._collapsePaneString); It's weird having both pane-opened and pane-collapsed attributes. Why not use a "checked" attribute, or "open"? Better yet, why not use just a single one everywhere? ::: browser/devtools/debugger/debugger.xul @@ +362,5 @@ > tooltiptext="&debuggerUI.sources.blackBoxTooltip;" > command="blackBoxCommand"/> > <toolbarbutton id="pretty-print" > class="devtools-toolbarbutton devtools-monospace" > + actlikeimage="true" A text-as-image attribute sounds better to me ::: browser/devtools/framework/toolbox.xul @@ +69,5 @@ > <toolbar class="devtools-tabbar"> > <hbox id="toolbox-picker-container" /> > <hbox id="toolbox-tabs" flex="1" /> > <hbox id="toolbox-buttons" pack="end"/> > + <vbox id="toolbox-controls-separator" class="devtools-separator"/> Why do we now have both toolbox-controls-separator and devtools-separator? Why do we need this distinction? ::: browser/themes/shared/devtools/toolbars.inc.css @@ +46,3 @@ > } > > +.devtools-toolbarbutton[label][big] { Hmm, I don't see why a .devtools-toolbar ancestor is specified above, and not here (nor below). Either enforce a .devtools-toolbar ancestor everywhere, or don't, otherwise things get weird and inconsistent.

Comment on attachment 8431559 [details] [diff] [review] thin-toolbar-part1.patch Review of attachment 8431559 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/shared/devtools/netmonitor.inc.css @@ +74,5 @@ > .requests-menu-header-button { > -moz-appearance: none; > background: none; > min-width: 1px; > + min-height: 24px; See my previous observation in the other review on how the netmonitor toolbars are of different heights. ::: browser/themes/shared/devtools/toolbars.inc.css @@ +16,3 @@ > border-bottom-width: 1px; > border-bottom-style: solid; > + min-height: 24px; Have you tested this height with different font sizes, especially on Linux? How about when using 200% UI elements and/or font-size on Windows? Would 1em be better in this case than a fixed pixel size? ::: browser/themes/shared/devtools/widgets.inc.css @@ +61,5 @@ > > .breadcrumbs-widget-container { > -moz-margin-end: 3px; > + max-height: 24px; /* Set max-height for proper sizing on linux */ > + height: 24px; /* Set height to prevent starting small waiting for content */ Wouldn't just setting height be sufficient? Is height + max-height required? Weird, maybe some flex needs to be removed or a align/pack attribute added to a parent element instead.

Comment on attachment 8437023 [details] [diff] [review] thin-toolbar-part2.patch This was supposed to be a f+, not a f?

Comment on attachment 8437023 [details] [diff] [review] thin-toolbar-part2.patch Review of attachment 8437023 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/canvasdebugger/canvasdebugger.xul @@ +48,5 @@ > pack="center" > flex="1"> > <button id="reload-notice-button" > class="devtools-toolbarbutton" > + big="true" I seem to remember a mozilla css style guide somewhere recommending to use attributes instead of class names for this type of thing, but I'm happy to switch it to a class name. I couldn't really think of a better name at the time - it is bigger than a normal button and has borders. Maybe devtools-standalone-button or just devtools-button or something like that? The only issue is that may imply that you don't *also* need to devtools-toolbarbutton class applied (which you currently do). Thinking about it more, I guess I could make it .devtools-button and remove the devtools-toolbarbutton class altogether ::: browser/devtools/debugger/debugger.xul @@ +362,5 @@ > tooltiptext="&debuggerUI.sources.blackBoxTooltip;" > command="blackBoxCommand"/> > <toolbarbutton id="pretty-print" > class="devtools-toolbarbutton devtools-monospace" > + actlikeimage="true" Agreed ::: browser/devtools/framework/toolbox.xul @@ +69,5 @@ > <toolbar class="devtools-tabbar"> > <hbox id="toolbox-picker-container" /> > <hbox id="toolbox-tabs" flex="1" /> > <hbox id="toolbox-buttons" pack="end"/> > + <vbox id="toolbox-controls-separator" class="devtools-separator"/> toolbox-controls-separator is accessed by ID from js (in toolbox.js). So devtools-separator is the class to use if you want a separator element. ::: browser/themes/shared/devtools/toolbars.inc.css @@ +46,3 @@ > } > > +.devtools-toolbarbutton[label][big] { Buttons that are children of the devtools-toolbar do not have borders, while the [big] ones do (and may or may not live underneath a toolbar).

(In reply to Victor Porof [:vporof][:vp] from comment #50) > Comment on attachment 8437023 [details] [diff] [review] > thin-toolbar-part2.patch > > Review of attachment 8437023 [details] [diff] [review]: > ----------------------------------------------------------------- > 6. The autocompletion popup is the only remaining UI element with rounded > borders [3]. Followup material, but I really think we ought to remove the > border radius there. The tooltip widget and the search box still have rounded corners.

Comment on attachment 8431559 [details] [diff] [review] thin-toolbar-part1.patch Review of attachment 8431559 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/shared/devtools/netmonitor.inc.css @@ +74,5 @@ > .requests-menu-header-button { > -moz-appearance: none; > background: none; > min-width: 1px; > + min-height: 24px; Looks like this issue was introduced in part 2 ::: browser/themes/shared/devtools/toolbars.inc.css @@ +16,3 @@ > border-bottom-width: 1px; > border-bottom-style: solid; > + min-height: 24px; I will check on Linux. Pixel sizes seem to zoom quite well these days - and it makes it easier for consistency with units across other files (and for things like the buttons and breadcrumbs that use some pixel offset). Before this patch, the toolbar's height was implied by containing elements that were set with px height which seems like it would have the same issues ::: browser/themes/shared/devtools/widgets.inc.css @@ +61,5 @@ > > .breadcrumbs-widget-container { > -moz-margin-end: 3px; > + max-height: 24px; /* Set max-height for proper sizing on linux */ > + height: 24px; /* Set height to prevent starting small waiting for content */ This was due to an issue on Linux (https://bugzilla.mozilla.org/show_bug.cgi?id=936421#c29). I wasn't able to find another solution at the time, but surely there is something else that could be done

OK, I've updated the heights to be more consistent, and renamed [actlikeimage] to [text-as-image] and renamed [big] to [standalone]. I've also added support for [standalone] buttons that are icon buttons (like the snapshot button in the canvas debugger). Also, removed pane-opened as it wasn't needed after all. I've pushed to try to test on Linux. Windows actually looked pretty good from my last build: https://tbpl.mozilla.org/?tree=Try&rev=97923d9ffd03. Inline feedback below: > 1. Certain scope headers in the variables view don't have a bottom border > [0]. I'm not seeing this on http://fiddle.jshell.net/bgrins/kCSDW/show/. Maybe one of the fixes below picked this up? Let me know if you are still seeing this > 2. Text and arrows don't appear to be vertically aligned anymore [0] (see > "Function scope"). I'm not really seeing it well, but Bug 949462 is swapping out those twistys so maybe that will update this. > 3. The "Sources", "Call Stack", "Variables" etc. toolbar buttons are > disproportionately tall vs. the every other toolbar and toolbarbutton. Fixed by removing a .devtools-sidebar-tabs > tabs > tab selector in debugger.inc.css > 4. In the network monitor, the table header is a few pixels taller than the > tabs in the inspection pane [1]. Fixed by removing an extra px I had missed when adding margins to the toolbarbuttons > 5. In the webconsole, the logs toolbar is a few pixels taller than the > object inspector search toolbar [2]. Fixed by removing a .variables-view-searchinput selector in widgets.inc.css.

A comparison of the toolbox with and without the patches applied on Ubunutu

Had a few rejects trying this out. I fixed them myself, but just letting you know. Hunk #1 FAILED at 28 1 out of 1 hunks FAILED -- saving rejects to file browser/themes/shared/devtools/webaudioeditor.inc.css.rej patching file browser/themes/shared/devtools/widgets.inc.css Hunk #1 FAILED at 872 1 out of 1 hunks FAILED -- saving rejects to file browser/themes/shared/devtools/widgets.inc.css.rej Also, make sure you synchronize with bug 984051, which adds a "standalone" button to the netmonitor.

Comment on attachment 8437904 [details] [diff] [review] thin-toolbar-part2.patch Review of attachment 8437904 [details] [diff] [review]: ----------------------------------------------------------------- Just a few additional comments: * Is it just me, or does it look like the text inside the breadcrumbs buttons isn't vertically centered? Compared to the toolbar tabs, it's 1 or 2 px lower than the baseline in the tabs. * The @media toolbar in the Style Editor has a weirdly placed label inside of it [4] * The webconsole filter/clear buttons look really weird when the toolbox is narrow [5] Everything else is gorgeous though!

Good catch on the console buttons - I've updated them to not clear out the text on the clear button and to make the really big circles a bit smaller

> * Is it just me, or does it look like the text inside the breadcrumbs > buttons isn't vertically centered? Compared to the toolbar tabs, it's 1 or 2 > px lower than the baseline in the tabs. Maybe? I unscientifically drew some lines and if you look at the 'l' characters maybe it is off by a px or two.

Rebased and fixed a couple of issues from Comment 63. Going to fold the two patches together for landing

Folded and built on top of Bug 1023666 to update the new reload button in the netmonitor.

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=666b0c51e70c

Going to land this in front of Bug 984051 and add the standalone attributes there

(In reply to Brian Grinstead [:bgrins] from comment #71) > Going to land this in front of Bug 984051 and add the standalone attributes > there Never mind, that landed already

Backed out changeset 9fd9c035a76a (bug 942292) for making browser_canvas-frontend-slider-02.js fail 30-40% of the time on opt linux runs: https://tbpl.mozilla.org/php/getParsedLog.php?id=41680601&tree=Fx-Team https://tbpl.mozilla.org/php/getParsedLog.php?id=41685001&tree=Fx-Team https://tbpl.mozilla.org/php/getParsedLog.php?id=41688596&tree=Fx-Team https://tbpl.mozilla.org/php/getParsedLog.php?id=41689162&tree=Fx-Team https://tbpl.mozilla.org/php/getParsedLog.php?id=41689109&tree=Fx-Team https://tbpl.mozilla.org/php/getParsedLog.php?id=41685415&tree=Fx-Team remote: https://hg.mozilla.org/integration/fx-team/rev/2cb7a1c95803

(In reply to Ed Morley [:edmorley UTC+0] from comment #74) > Backed out changeset 9fd9c035a76a (bug 942292) for making > browser_canvas-frontend-slider-02.js fail 30-40% of the time on opt linux > runs: > https://tbpl.mozilla.org/php/getParsedLog.php?id=41680601&tree=Fx-Team > https://tbpl.mozilla.org/php/getParsedLog.php?id=41685001&tree=Fx-Team > https://tbpl.mozilla.org/php/getParsedLog.php?id=41688596&tree=Fx-Team > https://tbpl.mozilla.org/php/getParsedLog.php?id=41689162&tree=Fx-Team > https://tbpl.mozilla.org/php/getParsedLog.php?id=41689109&tree=Fx-Team > https://tbpl.mozilla.org/php/getParsedLog.php?id=41685415&tree=Fx-Team > > remote: https://hg.mozilla.org/integration/fx-team/rev/2cb7a1c95803 I don't understand how this patch would trigger that error (this is CSS/HTML changes only). Still, I have a try push where I've excluded anything that was touching the canvas debugger files to see if that resolves it: https://tbpl.mozilla.org/?tree=Try&rev=ef80a95ad18d. I've noticed looking through tbpl that this same error has been coming up quite a bit lately (on different canvas debugger tests), including on the push where this was backed out: https://tbpl.mozilla.org/?tree=Fx-Team&rev=2cb7a1c95803 and the push before this originally landed: https://tbpl.mozilla.org/?tree=Fx-Team&rev=d3d04ca08d9c. The actual error here is: 07:57:21 INFO - Message: TypeError: Not enough arguments to CanvasRenderingContext2D.fillRect. 07:57:21 INFO - Stack: 07:57:21 INFO - ContextUtils.replayAnimationFrame@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/canvas.js:622:11

Looks like something that landed a bit before this caused Bug 1025118. Once the tree reopens this should be safe to land

I re-landed this along with the backout for bug 1025118. https://hg.mozilla.org/integration/fx-team/rev/a981121ef93b