Closed
Bug 1157944
Opened 9 years ago
Closed 9 years ago
console.dir tree output obscures the next console call
Categories
(DevTools :: Console, defect, P2)
Tracking
(firefox40 verified)
VERIFIED
FIXED
Firefox 40
Tracking | Status | |
---|---|---|
firefox40 | --- | verified |
People
(Reporter: canuckistani, Assigned: jfong)
References
Details
(Whiteboard: [polish-backlog][difficulty=easy][bugday-20150610])
Attachments
(5 files, 2 obsolete files)
45.20 KB,
image/png
|
Details | |
23.08 KB,
image/png
|
Details | |
1.90 KB,
patch
|
bgrins
:
review+
|
Details | Diff | Splinter Review |
863 bytes,
patch
|
Details | Diff | Splinter Review | |
37.10 KB,
image/png
|
Details |
STR: 1. in about:config, set devtools.toolbox.footer.height to 250 (tested on MBAir screen) 2. load a page in a tab, open the console 3. run console.dir(window), the tree widget produced by console.dir() should extend below the bottom of the console output view 4. run 1+1 The output of 1+1 should be below the console.dir tree, instead it is drawn over it, see screenshot.
Updated•9 years ago
|
Whiteboard: [devedition-40][difficulty=easy]
Updated•9 years ago
|
Priority: -- → P2
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jfong
Assignee | ||
Comment 1•9 years ago
|
||
Uploading screenshot next.
Attachment #8600458 -
Flags: review?(bgrinstead)
Assignee | ||
Comment 2•9 years ago
|
||
After fix results.
Comment 3•9 years ago
|
||
Comment on attachment 8600458 [details] [diff] [review] Bug1157944.patch Review of attachment 8600458 [details] [diff] [review]: ----------------------------------------------------------------- I'm still seeing the problem with this patch applied when the console is very small. It looks like there's an inline height on `.inlined-variables-view .message-body` that is wrong either with or without this patch applied (and seems dependent on the current console size at the time of logging). Most likely you'll have to find and fix whatever is setting that height (maybe it's clamping the value based on the current console size?). This variable looks suspicious: https://dxr.mozilla.org/mozilla-central/search?q=CONSOLE_DIR_VIEW_HEIGHT&redirect=true
Attachment #8600458 -
Flags: review?(bgrinstead) → review-
Assignee | ||
Comment 4•9 years ago
|
||
Hmmm this seems to work when I resize the console to a really short height. Let me know if that works?
Attachment #8600458 -
Attachment is obsolete: true
Attachment #8600496 -
Flags: review?(bgrinstead)
Comment 5•9 years ago
|
||
Comment on attachment 8600496 [details] [diff] [review] Bug1157944.patch Review of attachment 8600496 [details] [diff] [review]: ----------------------------------------------------------------- We discussed another solution
Attachment #8600496 -
Flags: review?(bgrinstead)
Assignee | ||
Comment 6•9 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ef9caebde7e
Attachment #8600496 -
Attachment is obsolete: true
Attachment #8600951 -
Flags: review?(bgrinstead)
Comment 7•9 years ago
|
||
Comment on attachment 8600951 [details] [diff] [review] Bug1157944.patch Review of attachment 8600951 [details] [diff] [review]: ----------------------------------------------------------------- As discussed, the tradeoff here is that on a very large display, console.dir output size will not be increased to scale with the screen size. It may be possible to target resizing the frame only using aView.window.frameElement from inside the call to openVariablesView. However, I'm not sure that picking an arbitrary height is the best way to show more content immediately. I'd prefer we go with this so there is a uniform height for all calls to console.dir() and possibly allow the user to resize these messages themselves (in a follow up)
Attachment #8600951 -
Flags: review?(bgrinstead) → review+
Comment 8•9 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #7) > I'd prefer we go with this so there is a uniform height for > all calls to console.dir() and possibly allow the user to resize these > messages themselves (in a follow up) This could be done via CSS only using something like this: .inlined-variables-view .message-body { resize: vertical; overflow: auto; min-height: 100px; }
Updated•9 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 9•9 years ago
|
||
Here is the patch with the vertical resize fix. The only issue is that the vertical scroll within .message-body plus the scroll in the iframe can cause the iframe's top and bottom edges to be masked (where the top or bottom borders are above/below the height of the .message-body wrapper. This causes the iframe bleed into the white background unless your mouse is always hovering over the affected area so that the blue background color takes over. I'll attach a screenshot next.
Flags: needinfo?(bgrinstead)
Assignee | ||
Comment 10•9 years ago
|
||
Flags: needinfo?(bgrinstead)
Assignee | ||
Comment 11•9 years ago
|
||
Comment on attachment 8600993 [details]
Bug_1157944-border-scroll.png
Accidentally cleared needinfo
Flags: needinfo?(bgrinstead)
Comment 12•9 years ago
|
||
(In reply to Jen Fong-Adwent [:ednapiranha] from comment #9) > Here is the patch with the vertical resize fix. The only issue is that the > vertical scroll within .message-body plus the scroll in the iframe can cause > the iframe's top and bottom edges to be masked (where the top or bottom > borders are above/below the height of the .message-body wrapper. This causes > the iframe bleed into the white background unless your mouse is always > hovering over the affected area so that the blue background color takes > over. I'll attach a screenshot next. Let's land the part 1 patch here and file a follow up bug for the CSS change
Flags: needinfo?(bgrinstead)
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 13•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/474d3b594af4
Keywords: checkin-needed
Comment 14•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/474d3b594af4
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Updated•9 years ago
|
QA Whiteboard: [good first verify]
Comment 15•9 years ago
|
||
I've followed the instructions from comment 0 and reproduced the bug with Firefox Nightly 40.0a1(20150423030204) and latest Beta 39.0b4(20150609130336) on Windows 7 x64. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:39.0) Gecko/20100101 Firefox/39.0 Verified as fixed with latest Aurora 40.0a2(20150610004004) and latest Nightly 41.0a1(20150610030209) Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0 Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:41.0) Gecko/20100101 Firefox/41.0
Comment 16•9 years ago
|
||
[bugday-20150610]
Comment 17•9 years ago
|
||
I have reproduced this bug with Nightly 40.0a1 (2015-04-23) with instruction from comment 0 and Linux x64. Verified as fixed with: latest Nightly 41.0a1 (2015-06-10) (Build ID: 20150610030209), Firefox Aurora 40.0a2 (2015-06-10) (Build ID: 20150610004004) Mozilla/5.0 (X11; Linux x86_64; rv:41.0) Gecko/20100101 Firefox/41.0 Mozilla/5.0 (X11; Linux x86_64; rv:40.0) Gecko/20100101 Firefox/40.0 [bugday-20150610]
Updated•9 years ago
|
Status: RESOLVED → VERIFIED
QA Whiteboard: [good first verify] → [good first verify][bugday-20150610]
Whiteboard: [devedition-40][difficulty=easy] → [devedition-40][difficulty=easy][bugday-20150610]
Reporter | ||
Updated•9 years ago
|
Whiteboard: [devedition-40][difficulty=easy][bugday-20150610] → [polish-backlog][difficulty=easy][bugday-20150610]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•