Closed Bug 1122662 Opened 10 years ago Closed 9 years ago

Overview graphs don't refresh when the browser window is resized

Categories

(DevTools :: Performance Tools (Profiler/Timeline), defect, P2)

defect

Tracking

(firefox40 verified, firefox41 fixed)

VERIFIED FIXED
Firefox 41
Tracking Status
firefox40 --- verified
firefox41 --- fixed

People

(Reporter: vporof, Assigned: bgrins)

References

Details

Attachments

(1 file, 1 obsolete file)

STR: 1. Start recording 2. Finish recording 3. Make the browser window narrower Overview graphs should redraw.
Blocks: perf-tool-v2
Jordan practically begged me to work on this
Assignee: nobody → jsantell
Status: NEW → ASSIGNED
introduced via markers/memory timelines in bug 1077457, bafb52753752d4c49ead17cf045d239675811af4
No longer blocks: perf-tool-v2
Blocks: perf-tool-v2
No longer blocks: enable-perf-tool
Blocks: perf-tool-papercuts
No longer blocks: perf-tool-v2
Assignee: jsantell → nobody
Assignee: nobody → vporof
Priority: -- → P2
Attached patch graph-resizer.patch (obsolete) (deleted) — Splinter Review
Took a lot of investigation, but here is what appears to be happening: * When the window gets resized to be larger, the iframes get a new width (based on the boundingClientRect of their parent) * When the window gets resized to be smaller, the iframes old width (the old window size) force the boundingClientRect of it's parent to stay at the previous size. I tried a lot of things to track this down, but the simplest one was to convert the iframe parents to be display: flex and then set the iframe's width to 100% in CSS. *shrug* A separate problem is that some of the long labels in the <deck> are taking up a lot of space (> 500px) and this causes any measurement of the parent element to be wrong since the performance-view-content container gets enlarged from this. This was amazingly hard to track down, and I ended up setting by switching to textContent for the label and then using the vw unit in CSS for max-width. Victor and/or Jordan, please apply the patch and double check that this fixes the issue you are seeing.
Assignee: vporof → bgrinstead
Attachment #8608417 - Flags: review?(vporof)
Attachment #8608417 - Flags: feedback?(jsantell)
Comment on attachment 8608417 [details] [diff] [review] graph-resizer.patch Review of attachment 8608417 [details] [diff] [review]: ----------------------------------------------------------------- Oh my, this is great! Overview graphs, flame graph all resize correctly when resizing the window. They all kinda rerender and pop-out at different sizes a bit (probably a canvas thing), but that's fine (and/or can be handled in the future), but other than that, looks great! The waterfall doesn't rerender and resize, but again that's another bug somewhere. Thanks Brian!
Attachment #8608417 - Flags: feedback?(jsantell) → feedback+
Comment on attachment 8608417 [details] [diff] [review] graph-resizer.patch Review of attachment 8608417 [details] [diff] [review]: ----------------------------------------------------------------- Teach me your ways. ::: browser/themes/shared/devtools/performance.inc.css @@ +157,5 @@ > +/* These rules prevent graph frames from not being able to shrink once expanded > + by not letting a width attribute set on them expand the boundingClientRect > + of the parent elements. See Bug 1122662. */ > +#overview-pane > hbox, > +#js-flamegraph-view { Targeting the js-flamegraph-view is oddly specific, and I can imagine this can happen with all the other decks as well, in the future. Make this selector a bit more generic. @@ +162,5 @@ > + display: flex; > +} > + > +#overview-pane iframe, > +#js-flamegraph-view iframe { Ditto.
Attachment #8608417 - Flags: review?(vporof) → review+
Attached patch graph-resizer.patch (deleted) — Splinter Review
Moved the graph styling out of specific CSS selectors and into the AbstractCanvasGraph.createIframe call. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=52f83fafb43b
Attachment #8608417 - Attachment is obsolete: true
Attachment #8609091 - Flags: review+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 41
Flags: qe-verify+
Comment on attachment 8609091 [details] [diff] [review] graph-resizer.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
Attachment #8609091 - Flags: approval-mozilla-aurora?
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 8609091 [details] [diff] [review] graph-resizer.patch Change approved to skip one train as part of the spring campaign.
Attachment #8609091 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed on Aurora 40.0a2 (2015-06-04), using Windows 7 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.8.5. The Waterfall, Call Tree and Flamechart are properly resized according to browser window size.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: