Bug 112338

Summary:

Web Inspector: [DataGrid] Specify columns with array of descriptors.

Product:

WebKit

Reporter:

Eugene Klyuchnikov <eustas>

Component:

Web Inspector (Deprecated)

Assignee:

Eugene Klyuchnikov <eustas>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

apavlov, keishi, loislo, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit.review.bot, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none

Eugene Klyuchnikov

Reported 2013-03-14 03:18:59 PDT

Currently columns are specified by Object that maps column identifiers to column descriptors. Iteration order over Object keys is not guaranteed. Array should be used to specify column order.

Attachments
Patch (39.52 KB, patch) 2013-03-14 03:22 PDT, Eugene Klyuchnikov	no flags	Details Formatted Diff Diff
Patch (40.33 KB, patch) 2013-03-14 07:03 PDT, Eugene Klyuchnikov	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Eugene Klyuchnikov

Comment 1 2013-03-14 03:22:19 PDT

Created attachment 193100 [details] Patch

Pavel Feldman

Comment 2 2013-03-14 05:30:58 PDT

Comment on attachment 193100 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193100&action=review > Source/WebCore/inspector/front-end/DataGrid.js:29 > + * @param {!Array.<!WebInspector.DataGrid.ColumnDescriptor>} columns Lets name it columnsArray > Source/WebCore/inspector/front-end/DataGrid.js:84 > + column.ordinal = i; Why do you need ordinals now? > Source/WebCore/inspector/front-end/DataGrid.js:-117 > - columnGroup.span = this._columnCount; Where did this go? > Source/WebCore/inspector/front-end/DataGrid.js:-125 > - var fillerRow = document.createElement("tr"); Where did this go?

Eugene Klyuchnikov

Comment 3 2013-03-14 06:51:43 PDT

Comment on attachment 193100 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193100&action=review >> Source/WebCore/inspector/front-end/DataGrid.js:29 >> + * @param {!Array.<!WebInspector.DataGrid.ColumnDescriptor>} columns > > Lets name it columnsArray Ok >> Source/WebCore/inspector/front-end/DataGrid.js:84 >> + column.ordinal = i; > > Why do you need ordinals now? Ordinals are still handy for editing/navigation. I'll cut it and show how it looks like without ordinals. >> Source/WebCore/inspector/front-end/DataGrid.js:-117 >> - columnGroup.span = this._columnCount; > > Where did this go? See :74 >> Source/WebCore/inspector/front-end/DataGrid.js:-125 >> - var fillerRow = document.createElement("tr"); > > Where did this go? See :76-77, :126

Eugene Klyuchnikov

Comment 4 2013-03-14 06:57:40 PDT

.ordinal is used for custom column resize in NetworkPanel. IMHO removing .ordinal should be a subject of another patch.

Eugene Klyuchnikov

Comment 5 2013-03-14 07:03:17 PDT

Created attachment 193117 [details] Patch

WebKit Review Bot

Comment 6 2013-03-14 22:31:17 PDT

Comment on attachment 193117 [details] Patch Clearing flags on attachment: 193117 Committed r145875: <http://trac.webkit.org/changeset/145875>

WebKit Review Bot

Comment 7 2013-03-14 22:31:20 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.