1514815 - Console output doesn't stay stay scrolled to bottom

Status: VERIFIED FIXED

(DevTools :: Console, defect, P1)

Description

[:Harald Kirschner :digitarald]

Attachment: doesnt-stay-scrolled-down.gif

STR:
- Open https://nchevobbe.github.io/demo/console-test-app.html
- Click "Log Error" button until the console is filled, then some more

ER: Console output stays scrolled to bottom, always showing the last message.
AR: Scroll position isn't updated and position stays fixed at one position

Async resolution of stack traces doesn't update scroll position

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1514815 - Add an `onReady` prop to the SmartTrace component; r=bgrins!.

Since the component renders asynchronously, consumers might
want to hook up to the actual first rendering.
We provide an `onRender` prop that will be called once, when
the component is ready.

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1514815 - Keep console scrolled to bottom when rendering SmartTrace; r=bgrins.

Depends on D14999

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/209bdfcecdc9
Add an `onReady` prop to the SmartTrace component; r=bgrins.
https://hg.mozilla.org/integration/autoland/rev/d822219ea9bc
Keep console scrolled to bottom when rendering SmartTrace; r=bgrins.

Comment 4

[Arthur Iakab [arthur_iakab]]

Backed out 2 changesets (bug 1514815) for failing devtools on browser_console_consolejsm_output.js CLOSED TREE

Backout revision https://hg.mozilla.org/integration/autoland/rev/ec5a6528e1e159ae2987704e3c74db2b05e33af3

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=218116400&revision=d822219ea9bc8105d783d5fc994f1b830c57a9f2

Failure log https://treeherder.mozilla.org/logviewer.html#?job_id=218116400&repo=autoland

Nicolas Chevobbe could you please take a look?

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1514815 - Add an `onReady` prop to the SmartTrace component; r=bgrins!.

Since the component renders asynchronously, consumers might
want to hook up to the actual first rendering.
We provide an `onRender` prop that will be called once, when
the component is ready.

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1514815 - Keep console scrolled to bottom when rendering SmartTrace; r=bgrins.

Depends on D15098

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

forgot to push the fix I made locally …

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c1043d77940a0df1853498c64f809320c0415a87

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/851d0b820ac2
Add an `onReady` prop to the SmartTrace component; r=bgrins.
https://hg.mozilla.org/integration/autoland/rev/0381e284d33b
Keep console scrolled to bottom when rendering SmartTrace; r=bgrins.

Comment 10

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/851d0b820ac2
https://hg.mozilla.org/mozilla-central/rev/0381e284d33b

Comment 11

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 9032512 [details]
Bug 1514815 - Keep console scrolled to bottom when rendering SmartTrace; r=bgrins.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1508549

User impact if declined: Console won't stay scrolled to the bottom if a stacktrace is logged at some point (might be an error message or a console.trace), which is not ideal because we want to talk about how we made stacktrace better in 65.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: 1. Go to https://nchevobbe.github.io/demo/console-test-app.html
2. Open the console
3. Click on the "Log Error" button in content multiple times, so the console output has a scrollbar
4. Click on the "Log Error" button again a few times

Expected result: 
The console should always stay scroll to the bottom.

List of other uplifts needed: other patch in the same bug

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Devtools only, thoroughly tested manually, has a mochitest

String changes made/needed:

Comment 12

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 9032488 [details]
Bug 1514815 - Add an `onReady` prop to the SmartTrace component; r=bgrins!.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: see other patch uplift request

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): 

String changes made/needed:

Comment 13

[Ryan VanderMeulen (PTO, back July 27)]

Comment on attachment 9032488 [details]
Bug 1514815 - Add an `onReady` prop to the SmartTrace component; r=bgrins!.

[Triage Comment]
Ensures that the console stays scrolled to the bottom when a stacktrace is logged. Thanks for including new tests. Approved for 65.0b7.

Comment 14

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/releases/mozilla-beta/rev/84ce2d096ccc
https://hg.mozilla.org/releases/mozilla-beta/rev/f615c123ed56

Comment 15

[Kanchan Kumari QA]

I could reproduce this issue on 
Version 	66.0a1
Build ID 	20181217000148


I verified this bug fix on latest Firefox Nightly 66.0a1 (2018-12-26)and the Firefox Beta 65.0b7 (Build ID 20181226223516) with Windows 7 (x64), Ubuntu 18.04 (x64) and macOS 10.13.6

Build ID 	20181226093642 	
User Agent 	Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:66.0) Gecko/20100101 Firefox/66.0


Build ID 	20181226215140
User Agent 	Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:66.0) Gecko/20100101 Firefox/66.0


Build ID     20181226215140
User Agent     Mozilla/5.0 (X11; Linux x86_64; rv:66.0) Gecko/20100101 Firefox/66.0


Version 	65.0b7
Build ID 	20181226223516