Bug 170090 - Web Inspector: WebSockets: Messages log should remain being scrolled to the bottom when a new message is added
Summary: Web Inspector: WebSockets: Messages log should remain being scrolled to the b...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nikita Vasilyev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-24 17:40 PDT by Nikita Vasilyev
Modified: 2017-03-29 18:06 PDT (History)
3 users (show)

See Also:


Attachments
Patch (6.42 KB, patch)
2017-03-27 17:00 PDT, Nikita Vasilyev
mattbaker: review+
Details | Formatted Diff | Diff
Patch (5.50 KB, patch)
2017-03-29 17:25 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2017-03-24 17:40:02 PDT
Adding a new message to WebSockets content view should scroll it to the bottom when WebSockets content view was scrolled to the bottom right before the new message being added.

In this regard, WebSockets content view should behave the same as the Console.
Comment 1 Nikita Vasilyev 2017-03-27 17:00:53 PDT
Created attachment 305527 [details]
Patch
Comment 2 Matt Baker 2017-03-27 18:29:03 PDT
Comment on attachment 305527 [details]
Patch

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

r=me, with some style suggestions.

> Source/WebInspectorUI/ChangeLog:16
> +        Batch WebSocketContentView DOM modifications using requestAnimationFrames.

"requestAnimationFrame" should be singular.

> Source/WebInspectorUI/UserInterface/Views/WebSocketContentView.js:103
> +        this._scheduledUpdateFramesIdentifier = requestAnimationFrame(() => this._updateFrames());

Utilities.js defines Object.onNextFrame, which does exactly this! You could probably switch to that and remove a bunch of boilerplate.

For an example see TreeElement.js:194.

> Source/WebInspectorUI/UserInterface/Views/WebSocketContentView.js:145
> +            isText: opcode === WebInspector.WebSocketResource.OpCodes.TextFrame,

Since its value is used above, `isText` could be a stand-alone variable, defined and initialized above `nodeText`:

let isText = opcode === WebInspector.WebSocketResource.OpCodes.TextFrame;
let nodeText = isText ? data : WebInspector.WebSocketContentView.textForOpcode(opcode);

let attributes = {isOutgoing, isText};
...
Comment 3 Nikita Vasilyev 2017-03-29 17:25:05 PDT
Created attachment 305813 [details]
Patch
Comment 4 WebKit Commit Bot 2017-03-29 18:06:34 PDT
Comment on attachment 305813 [details]
Patch

Clearing flags on attachment: 305813

Committed r214587: <http://trac.webkit.org/changeset/214587>
Comment 5 WebKit Commit Bot 2017-03-29 18:06:39 PDT
All reviewed patches have been landed.  Closing bug.