Bug 83122 - Web Inspector: make padding and client window width part of timeline calculator's state
Summary: Web Inspector: make padding and client window width part of timeline calculat...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Andrey Kosyakov
URL:
Keywords:
Depends on: 83146
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-04 01:09 PDT by Andrey Kosyakov
Modified: 2012-04-04 05:42 PDT (History)
12 users (show)

See Also:


Attachments
Patch (17.11 KB, patch)
2012-04-04 01:17 PDT, Andrey Kosyakov
no flags Details | Formatted Diff | Diff
Patch (17.95 KB, patch)
2012-04-04 01:44 PDT, Andrey Kosyakov
pfeldman: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Kosyakov 2012-04-04 01:09:36 PDT
We used to pass client width to calculator's computeBarGraphWindowPosition(), then adjust for possible padding on the left. This often required passing paddingLeft along with the calculator and lead to possible inconsistent values of width used. Also, this makes it difficult to reuse position calculation logic for a single position, not the entire bar.
This adds display window dimensions to calculator's state, so that it's sufficient to pass calculator around and trivial to implement position calculation for single time value.
Comment 1 Andrey Kosyakov 2012-04-04 01:17:47 PDT
Created attachment 135527 [details]
Patch
Comment 2 Andrey Kosyakov 2012-04-04 01:44:50 PDT
Created attachment 135528 [details]
Patch
Comment 3 Pavel Feldman 2012-04-04 02:09:08 PDT
Comment on attachment 135528 [details]
Patch

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

> Source/WebCore/inspector/front-end/TimelinePanel.js:735
> +    this._minWidth = 5;

Please use constants for these
Comment 4 Andrey Kosyakov 2012-04-04 02:27:40 PDT
Committed r113156: <http://trac.webkit.org/changeset/113156>
Comment 5 Philippe Normand 2012-04-04 03:21:01 PDT
(In reply to comment #4)
> Committed r113156: <http://trac.webkit.org/changeset/113156>

This patch broke 4 inspector tests on GTK...
http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release/r113156%20(21569)/results.html
Comment 6 Philippe Normand 2012-04-04 03:22:28 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > Committed r113156: <http://trac.webkit.org/changeset/113156>
> 
> This patch broke 4 inspector tests on GTK...
> http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release/r113156%20(21569)/results.html

https://bugs.webkit.org/show_bug.cgi?id=83136
Comment 8 Andrey Kosyakov 2012-04-04 05:42:14 PDT
Committed r113177: <http://trac.webkit.org/changeset/113177>