Still needs some more tests, which I haven't attached yet.

Make sure 'browser_toolbox_options.js' doesn't assume all tools are enabled while testing.

Looks good so far: https://tbpl.mozilla.org/?tree=Try&rev=84ee2ad67ab4

Comment on attachment 812676 [details] [diff] [review] webgl-frontend.patch Rob practically begged me to review this.

Comment on attachment 813193 [details] [diff] [review] webgl-fix-toolbox-test.patch And this.

Comment on attachment 813193 [details] [diff] [review] webgl-fix-toolbox-test.patch Probably no longer needed.

Comment on attachment 812676 [details] [diff] [review] webgl-frontend.patch Review of attachment 812676 [details] [diff] [review]: ----------------------------------------------------------------- Nice and easy-to-understand. ::: browser/devtools/shadereditor/panel.js @@ +20,5 @@ > + > +exports.ShaderEditorPanel = ShaderEditorPanel; > + > +ShaderEditorPanel.prototype = { > + open: function() { Any chance of open being called twice? Should we save the init promise and return it again in that case? @@ +29,5 @@ > + targetPromise = this.target.makeRemote(); > + } else { > + targetPromise = promise.resolve(this.target); > + } > + target.makeRemote() will do the right thing if the target has already been made remote, you shouldn't need this if statement. @@ +42,5 @@ > + this.emit("ready"); > + return this; > + }) > + .then(null, function onError(aReason) { > + Cu.reportError("ShaderEditorPanel open failed. " + Why not console.error? ::: browser/devtools/shadereditor/shadereditor.js @@ +191,5 @@ > + fs: fragmentShaderText > + }); > + } > + > + getShaders().then(getSources).then(showSources).then(null, Cu.reportError); Same console.error question

I felt a bit uneasy landing this as is, so I added a dozen more tests to be sure things won't regress in the future.

Try: https://tbpl.mozilla.org/?tree=Try&rev=8090f88e78c1

Is there a particular reason this is not mentioned in the Fx 27 release notes?

It never came up. I'd be more than happy if someone added it.

For relnote, remember that the Shader Editor is hidden by default thus needs to be enabled in the dev tools preferences. (I was reading a post on TNW[1] that wondered about the missing Shader Editor in the relnotes, speculating it may not be ready for beta/release. Maybe somebody can reach out to them?)

(In reply to Florian Bender from comment #14) > For relnote, remember that the Shader Editor is hidden by default thus needs > to be enabled in the dev tools preferences. > > (I was reading a post on TNW[1] that wondered about the missing Shader > Editor in the relnotes, speculating it may not be ready for beta/release. > Maybe somebody can reach out to them?) The Shader Editor is hidden by default ("disabled" is a too strong word) because most Web content out there doesn't contain WebGL, and horizontal estate in the Toolbox tabbar isn't cheap, so we're avoiding showing a tab which won't be primary to web developers. Web developers who regularly deal with WebGL can choose show the tab via the Options panel in the Toolbox. We have bug 937725 which plans to more gracefully deal with this situation. I'm not sure if adding a relnote would make more sense after that bug is fixed, or now, with a "can be shown via the Toolbox options tab" addendum. Either way is fine by me, but bug 937725 probably won't be getting attention very soon.

The Shader Editor is very nice, and is worthy of a nice little icon for itself instead of using the styleeditor icon.

Thanks! :) Bug 938290.

Firefox 27 has been released without this in the release notes. Maybe we could add this in the release notes when it will be displayed by default.