Bug 177914 - Web Inspector: When scrolled Network Table reduces the number of rows it may appear as blank
Summary: Web Inspector: When scrolled Network Table reduces the number of rows it may ...
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: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-10-04 21:28 PDT by Joseph Pecoraro
Modified: 2017-10-05 11:04 PDT (History)
5 users (show)

See Also:

Attachments
[PATCH] Proposed Fix (3.46 KB, patch)
2017-10-04 21:34 PDT, Joseph Pecoraro 		mattbaker: review+
Details | Formatted Diff | Diff
[PATCH] For Landing (3.43 KB, patch)
2017-10-04 23:06 PDT, Joseph Pecoraro 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2017-10-04 21:28:31 PDT 
When scrolled Network Table reduces the number of rows it may appear as blank

Steps to Reproduce:
1. Inspect apple.com
2. Show Network Tab
3. Reload
4. Scroll Network Tab Down
5. Select "Document" filter
  => expected to see 1 Document resource, instead see blank / broken table

Notes:
• The number of rows was reduced, but the Table's scrollTop was not reset to a reasonable value. This can happen any time the row count goes down so we should have a generic solution.
Comment 1 Radar WebKit Bug Importer 2017-10-04 21:28:54 PDT 
<rdar://problem/34827613>
Comment 2 Joseph Pecoraro 2017-10-04 21:34:25 PDT 
Created attachment 322777 [details]
[PATCH] Proposed Fix
Comment 3 Matt Baker 2017-10-04 21:55:35 PDT 
Comment on attachment 322777 [details]
[PATCH] Proposed Fix

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

r=me, with (optional) nits.

> Source/WebInspectorUI/UserInterface/Views/Table.js:897
> +        if (this._cachedScrollTop) {

Might as well use scrollTop from above, since scrollableOffsetHeight is used.

> Source/WebInspectorUI/UserInterface/Views/Table.js:901
> +                if (this._cachedScrollTop > maximumScrollTop) {

Ditto.

> Source/WebInspectorUI/UserInterface/Views/Table.js:950
> +        // of rows can fit on screen. Reset the scroll top.

Should this say "Reset the scroll top to zero", so that it's obvious that we didn't mean to set it to NaN?
Comment 4 Joseph Pecoraro 2017-10-04 23:06:38 PDT 
Created attachment 322787 [details]
[PATCH] For Landing
Comment 5 WebKit Commit Bot 2017-10-04 23:33:59 PDT 
Comment on attachment 322787 [details]
[PATCH] For Landing

Clearing flags on attachment: 322787

Committed r222899: <http://trac.webkit.org/changeset/222899>