Bug 191174

Summary: Web Inspector: Network: remove unnecessary media event tracking
Product: WebKit Reporter: Devin Rousso <hi>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, hi, inspector-bugzilla-changes, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 189773    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
[Image] After Patch is applied
none
Patch none

Devin Rousso
Reported 2018-11-01 15:27:13 PDT
There's no reason to show event "bubbles" for events that have nothing to do with network activity (e.g. "volumechanged").
Attachments
Patch (17.75 KB, patch)
2018-11-01 15:37 PDT, Devin Rousso
no flags
[Image] After Patch is applied (1.25 MB, image/png)
2018-11-01 15:37 PDT, Devin Rousso
no flags
Patch (18.46 KB, patch)
2018-11-01 16:47 PDT, Devin Rousso
no flags
Devin Rousso
Comment 1 2018-11-01 15:37:27 PDT
Devin Rousso
Comment 2 2018-11-01 15:37:59 PDT
Created attachment 353653 [details] [Image] After Patch is applied
Joseph Pecoraro
Comment 3 2018-11-01 16:06:45 PDT
Comment on attachment 353652 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353652&action=review r=me > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.css:91 > +.network-table .data-container .cell.name .range { > + font-style: italic; > +} I'm not sure I like the italics here. For tree elements we typically put a subtitle in a lighter gray color and not italicize. I'm not sure what would be best we can experiment. > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1103 > + setTimeout(() => { > + if (this._waterfallPopover) > + this._waterfallPopover.resize(); > + }); Why in a setTimeout? Should this be a debounce? > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1929 > + this._handleMousedownWaterfall(nodeEntry, popoverContentElement, (cell) => { > + let domEventElement = nodeEntry.domEventElements.get(domEvents[0]); It is weird to name this a `getTargetElementFunction` when it may also change the contents of the popover. I see why its happening though. Maybe a more generic `resizeAndPositionHandler`/`repositionAndResizeHandler` might be better?
Devin Rousso
Comment 4 2018-11-01 16:31:42 PDT
Comment on attachment 353652 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353652&action=review >> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1103 >> + }); > > Why in a setTimeout? Should this be a debounce? I want to call this function after the `WI.Table` has finished `layout`, as it's at that point that the elements are in the DOM. I think the better approach is to introduce another base function to `WI.View` that gets called once a subtree has finished `layout`.
Devin Rousso
Comment 5 2018-11-01 16:47:35 PDT
WebKit Commit Bot
Comment 6 2018-11-01 17:27:26 PDT
Comment on attachment 353661 [details] Patch Clearing flags on attachment: 353661 Committed r237708: <https://trac.webkit.org/changeset/237708>
WebKit Commit Bot
Comment 7 2018-11-01 17:27:27 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2018-11-01 17:28:29 PDT
Note You need to log in before you can comment on or make changes to this bug.