Summary: | Web Inspector: Use ECMAScript Numeric Separators for numbers with 5 or more digits | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nikita Vasilyev <nvasilyev> | ||||||
Component: | Web Inspector | Assignee: | Nikita Vasilyev <nvasilyev> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | inspector-bugzilla-changes, joepeck, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Nikita Vasilyev
2020-04-01 14:05:19 PDT
Created attachment 395201 [details]
Patch
Comment on attachment 395201 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=395201&action=review Nice! Some comments > Source/WebInspectorUI/UserInterface/Models/Geometry.js:569 > - epsilon = epsilon || 0.0001; > + epsilon = epsilon || 0.000_1; I don't like this one. But maybe that is just me? > Source/WebInspectorUI/UserInterface/Models/ProfileNode.js:161 > + if (this._selfTime < 0.000_1) Ditto > Source/WebInspectorUI/UserInterface/Models/TimelineRecording.js:539 > +WI.TimelineRecording.TimestampThresholdForLegacyAssumedMilliseconds = 1_420_099_200_000; // Date.parse("Jan 1, 2015"). Milliseconds since epoch. This is a very specific thing, not sure we even want to fake it being readable. > Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js:392 > + totalElement.textContent = Number.percentageString(percent === 1 ? percent : (percent - 0.000_5)); Ditto. Comment on attachment 395201 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=395201&action=review >> Source/WebInspectorUI/UserInterface/Models/Geometry.js:569 >> + epsilon = epsilon || 0.000_1; > > I don't like this one. But maybe that is just me? Yeah, I wasn't sure about this myself. I can revert. >> Source/WebInspectorUI/UserInterface/Models/TimelineRecording.js:539 >> +WI.TimelineRecording.TimestampThresholdForLegacyAssumedMilliseconds = 1_420_099_200_000; // Date.parse("Jan 1, 2015"). Milliseconds since epoch. > > This is a very specific thing, not sure we even want to fake it being readable. I agree. I simply found numbers with 5 or more digits. I'll revert this chunk if you think the rest is r+ worthy. > I agree. I simply found numbers with 5 or more digits. I'll revert this
> chunk if you think the rest is r+ worthy.
Yep
Created attachment 395209 [details]
Patch
Comment on attachment 395209 [details]
Patch
r=me
Committed r259368: <https://trac.webkit.org/changeset/259368> All reviewed patches have been landed. Closing bug and clearing flags on attachment 395209 [details]. |