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)

40 Branch
defect

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)

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.
Whiteboard: [devedition-40][difficulty=easy]
Priority: -- → P2
Assignee: nobody → jfong
Attached patch Bug1157944.patch (obsolete) — Splinter Review
Uploading screenshot next.
Attachment #8600458 - Flags: review?(bgrinstead)
Attached image console-dir.png
After fix results.
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-
Attached patch Bug1157944.patch (obsolete) — Splinter Review
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 on attachment 8600496 [details] [diff] [review]
Bug1157944.patch

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

We discussed another solution
Attachment #8600496 - Flags: review?(bgrinstead)
See Also: → 760876
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+
(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;
}
Status: NEW → ASSIGNED
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)
Flags: needinfo?(bgrinstead)
Comment on attachment 8600993 [details]
Bug_1157944-border-scroll.png

Accidentally cleared needinfo
Flags: needinfo?(bgrinstead)
(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)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/474d3b594af4
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
QA Whiteboard: [good first verify]
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
[bugday-20150610]
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]
Status: RESOLVED → VERIFIED
QA Whiteboard: [good first verify] → [good first verify][bugday-20150610]
Whiteboard: [devedition-40][difficulty=easy] → [devedition-40][difficulty=easy][bugday-20150610]
Whiteboard: [devedition-40][difficulty=easy][bugday-20150610] → [polish-backlog][difficulty=easy][bugday-20150610]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.