User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:47.0) Gecko/20100101 Firefox/47.0 Build ID: 20160208030244 Steps to reproduce: 1. Start any versions Firefox with new profile 2. Open Devtools > Performance 3. Check off "Record Framerate" option 4. Start and Stop recording performance Actual results: time-framerate is displayed and overlapped into detail-pane. Probably, hidden class does not enable. Expected results: time-framerate should be hidden if its option is disabled.

STR > 1. Start any versions Firefox with new profile > 2. Open Devtools > Performance + Start and Stop recording performance > 3. Check off "Record Framerate" option > 4. Start and Stop recording performance

Regression window pushlog_url: https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=0b01db3f7238c0848a0d18046602b0f8bc908dcb&tochange=1e64ee0dc934f5c92d10d495abfbf38acd1931d5

It's because there's a display flex set on a parent element which also has the 'hidden' class applied: https://hg.mozilla.org/integration/fx-team/rev/1e64ee0dc934#l2.19

Review commit: https://reviewboard.mozilla.org/r/35277/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/35277/

Comment on attachment 8720361 [details] MozReview Request: Bug 1246884 - Make sure hidden graphs display isn't overridden by Graphs.js;r=vporof https://reviewboard.mozilla.org/r/35277/#review32411 ::: devtools/client/performance/test/browser_perf-options-enable-framerate.js:18 (Diff revision 1) > + "PerformanceFront started with ticks recording."); Please put this into a new test. Starting and stopping a recording is notoriously heavy on try servers, and tests frequently time out if there's too much profiler action going on.

Should land this, maybe? Ping :bgrins.

Comment on attachment 8720361 [details] MozReview Request: Bug 1246884 - Make sure hidden graphs display isn't overridden by Graphs.js;r=vporof Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35277/diff/1-2/

Looks like it's been split into two tests in the meantime, let me know if you want this to be a third or if it's OK where I put it

https://reviewboard.mozilla.org/r/35277/#review37751 ::: devtools/client/performance/test/browser_perf-options-enable-framerate-01.js:26 (Diff revision 2) > > let { $, PerformanceController } = panel.panelWin; > > + // Start with default framerate setting (visible) > + yield startRecording(panel); > + yield stopRecording(panel); This test already starts/stops recording twice. Adding another third recording will likely cause this to intermittently time out on slow hardware and debug builds. Please create a new test :(

Comment on attachment 8720361 [details] MozReview Request: Bug 1246884 - Make sure hidden graphs display isn't overridden by Graphs.js;r=vporof Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35277/diff/2-3/

OK, split into a new test: https://treeherder.mozilla.org/#/jobs?repo=try&revision=50023140b72a

That try run isn't very happy looking :(

Huh... seems like running getComputedStyle on element in the test helper is causing exceptions to happen in the frontend, specifically this stack trace which is pointing to a place where the font is set on a canvas context (https://dxr.mozilla.org/mozilla-central/source/devtools/client/performance/modules/widgets/markers-overview.js#169): 30 INFO TEST-UNEXPECTED-FAIL | devtools/client/performance/test/browser_perf-overview-render-04.js | A promise chain failed to handle a rejection: - at resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/performance/modules/widgets/markers-overview.js:169 - Stack trace: JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/performance/modules/widgets/markers-overview.js :: MarkersOverview.prototype<.buildGraphImage :: line 169 JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/widgets/Graphs.js :: AbstractCanvasGraph.prototype.setData :: line 287 JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/performance/modules/widgets/graphs.js :: GraphsController.prototype.render< :: line 224 JS frame :: resource://gre/modules/Task.jsm :: TaskImpl_run :: line 319 JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: Handler.prototype.process :: line 937 JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: this.PromiseWalker.walkerLoop :: line 816

I'm going to close this bug to reflect reality. The new perf tool is being merged into the toolbox (tracked in Bug 1418044), so this won't get fixed.

> I'm going to close this bug to reflect reality. The new perf tool is being merged into the toolbox (tracked in Bug 1418044), so this won't get fixed. Just don't set your expectations to high: we're merging the perf tool but keeping it hidden behind a pref. We still have quite a few things to do before replacing the current perf tool and I'm still not sure how high a priority this will be next year.