Bug 67483 – Web Inspector: Console is always scrolled to the top

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Yury Semikhatsky]

Ustreaming Chromium bug: http://code.google.com/p/chromium/issues/detail?id=86832

What steps will reproduce the problem?
1. Open JS console.
2. Scroll to the bottom.
3. Go to the script tab.
4. Go back to the console.

What is the expected result?

The console should by default be scrolled to the bottom. That's where the newest information is and where new statements can be entered.


What happens instead?

Console always scrolls back to the top.

[Yury Semikhatsky]

Created attachment 106126 [details]
Patch

[Vsevolod Vlasov]

This does not look nice. Scroll changes when switching between console panel and console drawer are not smooth at all.

I suggest that we have the following behavior of scroll in console:

1) If we are not at the bottom of console, then store/restore scrollTop for possible each action.
2) If we are at the bottom of console, then keep this state until user scrolls explicitly.

I have a patch implementing this behavior ready and will upload it soon.

[Vsevolod Vlasov]

Created attachment 106166 [details]
Patch

[Pavel Feldman]

Comment on attachment 106166 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=106166&action=review

> Source/WebCore/inspector/front-end/ConsoleView.js:227
> +        WebInspector.View.prototype.show.call(this);

I remember caseq implementing View hierarchy that was moving the logic from show and hide to wasShown and willHide. Is this change in line with what he was doing?

> Source/WebCore/inspector/front-end/ConsoleView.js:254
> +        if (this._scrolledToBottom)

You might want to just override the original logic here so that you don't use this hybrid approach.

> Source/WebCore/inspector/front-end/ConsoleView.js:262
> +        this.restoreScrollPositions();

this._scrolledToBottom value might be out of date by this moment (user scrolled manually).

[Yury Semikhatsky]

Comment on attachment 106166 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=106166&action=review

> Source/WebCore/inspector/front-end/ConsoleView.js:255
> +            this._immediatelyScrollIntoView();

The console may have been cleared after the position was stored, in this case it doesn't make sense to restore scroller position if it's not bottom/top.

[Vsevolod Vlasov]

Created attachment 108033 [details]
Patch

[Vsevolod Vlasov]

> I remember caseq implementing View hierarchy that was moving the logic from show and hide to wasShown and willHide. Is this change in line with what he was doing?
New logic is added to wasShown now.
> 
> > Source/WebCore/inspector/front-end/ConsoleView.js:254
> > +        if (this._scrolledToBottom)
> 
> You might want to just override the original logic here so that you don't use this hybrid approach.
I don't understand your idea. Essentially console reuses typical store/restore scroll approach with an exception of scrolled-to-bottom case.
> 
> > Source/WebCore/inspector/front-end/ConsoleView.js:262
> > +        this.restoreScrollPositions();
> 
> this._scrolledToBottom value might be out of date by this moment (user scrolled manually).
The idea is that this._scrolledToBottom is updated before hide/resize starts, see _startStatusBarDragging() in Drawer.

(In reply to comment #5)
> (From update of attachment 106166 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=106166&action=review
> 
> > Source/WebCore/inspector/front-end/ConsoleView.js:255
> > +            this._immediatelyScrollIntoView();
> 
> The console may have been cleared after the position was stored, in this case it doesn't make sense to restore scroller position if it's not bottom/top.
Added call to storeScrollPositions in ConsoleCleared event handler.

[Pavel Feldman]

Comment on attachment 108033 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=108033&action=review

> Source/WebCore/inspector/front-end/ConsoleView.js:229
> +        WebInspector.View.prototype.wasShown.call(this);

This is a View's flaw. wasShown should be abstract.

> Source/WebCore/inspector/front-end/ConsoleView.js:284
> +        if (!this._scrollIntoViewTimer)

if (!this._isScrollIntoViewScheduled())

[Vsevolod Vlasov]

Committed r95716: <http://trac.webkit.org/changeset/95716>