Closed Bug 1239670 Opened 8 years ago Closed 8 years ago

Columns resized when expanding the scripts node of the census view

Categories

(DevTools :: Memory, defect, P2)

defect

Tracking

(firefox46 fixed)

RESOLVED FIXED
Firefox 46
Tracking Status
firefox46 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

STRs : 

- go to http://google.com
- open memory tool
- capture snapshot
- select view : Aggregate in memory tool
- expand the "scripts" node

Actual :
All columns are resized. Also if the scripts node is scrolled out, the columns will go back to their initial size.

Expected : 
Columns size should remain the same, or at least should not change later on while scrolling.
I think I isolated the issue in the following jsbin : 
http://jsbin.com/pugomuqaha/1/edit?html,output

The key, as you can see here there is a huge overflow even though the flex-containers are supposed to be 100x100. But if you add `width: 1px` (or any width really) to #item3 styles (-> http://jsbin.com/hagixemina/1/edit?html,output), then the dimension of the container is preserved.

This seems to occur only on Firefox.
Assignee: nobody → jdescottes
Has STR: --- → yes
Priority: -- → P2
We can and should work around this right now, but I'm wondering if this is also a layout bug? dholbert, maybe you can enlighten us? See comment 2 and its jsbin test case.
Flags: needinfo?(dholbert)
(In reply to Julian Descottes from comment #2)
> This seems to occur only on Firefox.

It occurs in Edge, as well.  This is an instance of bug 1043520, and it's correct per spec.  (Chrome doesn't match Firefox/Edge, but I'm pretty sure that's a chrome bug.)  Basically, the inner flex container's default "min-width:auto" makes refuse to be smaller than its min-content size, which includes the width of the verylongtext. (even in light of the overflow property being set on the verylongtext element)

"width: 1px" fixes things as noted in comment 2 because it sets an upper-limit for what "min-width:auto" can resolve to.

A cleaner solution is to set "min-width: 0" on the inner flex container.  That disables the min-width:auto behavior and allows it to be smaller than its min-content size.
Flags: needinfo?(dholbert)
(In reply to Daniel Holbert [:dholbert] from comment #4)
> (Chrome doesn't match Firefox/Edge, but I'm pretty sure
> that's a chrome bug.)

Specifically, it's https://code.google.com/p/chromium/issues/detail?id=487302  (though here we're talking about about horizontal sizing and that bug is about vertical sizing, but I think it's all the same)
Attached patch bug1239670.diffSplinter Review
Thanks for the detailed explanation dholbert!

Now when it comes to the memory table, we have two intermediary containers to update with "min-width: 0", #heap-view and #heap-view > .heap-view-panel .

@fitzgen : my guess is that a test for this would be quite fragile, but let me know if you want me to create one.
Attachment #8707999 - Flags: review?(nfitzgerald)
Comment on attachment 8707999 [details] [diff] [review]
bug1239670.diff

Review of attachment 8707999 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, testing this would be more pain than gain. Thanks for the patch!
Attachment #8707999 - Flags: review?(nfitzgerald) → review+
Thanks for the review!

Launched a try build for reference : https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f93bf03340f
Adding checkin needed as this is a minor CSS change.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f0b09da3f4ca
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
I had to back this out for being a possible cause of frequent devtools test failures in browser_profiler_tree-abstract-02.js

https://treeherder.mozilla.org/logviewer.html#?job_id=6636210&repo=fx-team
https://hg.mozilla.org/mozilla-central/rev/81a4830b685c
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Error is triggered in Linux x64 opt M-e10s dt6.

Triggered a try push up to this commit (this was the last of the 3 backedout changesets) : 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f11d34ef4f7

7/10 of e10s-dt6 failed on Linux x64 opt.

New try push ongoing with only this commit re-applied on top of a stable commit : https://treeherder.mozilla.org/#/jobs?repo=try&revision=218dd5652bec
Flags: needinfo?(jdescottes)
Commit f0b09da3f4ca did not trigger the intermittent tests and should be reapplied. 

(see Bug 1240368 for more info about the intermittent failures)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7cefd34bcf4f
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
I have successfully reproduced the bug in Nightly 46.0a1 (2016-01-14)windows 7 (32 bit) 
 
Verified as fixed with latest aurora 46.0a2 (2016-03-03)

build ID: 20160303004038 
Mozilla/5.0 (Windows NT 6.1; rv:46.0) Gecko/20100101 Firefox/46.0

[Test day-20160304]
[bugday-20160323]

Status: RESOLVED,FIXED -> VERIFIED

Comments: Test Not successful
STR:  clear.


Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Column size should not change even after expansion of attributes

Actual Results: 
Columns under the field scripts are shifting towards left side as I am expanding the attributes in scripts. Other field columns also shifting towards left side slightly
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.