Web Inspector: Make DirectoryContentView sortable

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Taiju Tsuiki]

DirectoryContentView should have sortable columns.

[Taiju Tsuiki]

Created attachment 150371 [details]
Patch

[Vsevolod Vlasov]

Comment on attachment 150371 [details]
Patch

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

Please make sure directories are correctly sorted.

> Source/WebCore/inspector/front-end/DirectoryContentView.js:139
> +        if (x._entry.isDirectory)

This means directories order would be undefined. We should sort them by name instead.

> Source/WebCore/inspector/front-end/DirectoryContentView.js:146
> +        return (x._metadata ? x._metadata.size || 0 : 0) - (y._metadata ? y._metadata.size || 0 : 0);

Is size defined for directories?

> Source/WebCore/inspector/front-end/DirectoryContentView.js:151
> +        return (x._metadata ? x._metadata.modificationTime : 0) - (y._metadata ? y._metadata.modificationTime : 0);

Is modification time defined for directories?

[Taiju Tsuiki]

Comment on attachment 150371 [details]
Patch

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

>> Source/WebCore/inspector/front-end/DirectoryContentView.js:139
>> +        if (x._entry.isDirectory)
> 
> This means directories order would be undefined. We should sort them by name instead.

I meant, all directories have same type.
Should we also sort other same-mimetype files by name?

>> Source/WebCore/inspector/front-end/DirectoryContentView.js:146
>> +        return (x._metadata ? x._metadata.size || 0 : 0) - (y._metadata ? y._metadata.size || 0 : 0);
> 
> Is size defined for directories?

No, it doesn't have size, currently.
Should we assign dummy value to directory size? It seems make thing simpler.

>> Source/WebCore/inspector/front-end/DirectoryContentView.js:151
>> +        return (x._metadata ? x._metadata.modificationTime : 0) - (y._metadata ? y._metadata.modificationTime : 0);
> 
> Is modification time defined for directories?

Yes, directories have modification time. It'll be updated when its content is created or removed.

[Vsevolod Vlasov]

(In reply to comment #3)
> (From update of attachment 150371 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150371&action=review
> 
> >> Source/WebCore/inspector/front-end/DirectoryContentView.js:139
> >> +        if (x._entry.isDirectory)
> > 
> > This means directories order would be undefined. We should sort them by name instead.
> 
> I meant, all directories have same type.
> Should we also sort other same-mimetype files by name?
> 
> >> Source/WebCore/inspector/front-end/DirectoryContentView.js:146
> >> +        return (x._metadata ? x._metadata.size || 0 : 0) - (y._metadata ? y._metadata.size || 0 : 0);
> > 
> > Is size defined for directories?
> 
> No, it doesn't have size, currently.
> Should we assign dummy value to directory size? It seems make thing simpler.
> 
> >> Source/WebCore/inspector/front-end/DirectoryContentView.js:151
> >> +        return (x._metadata ? x._metadata.modificationTime : 0) - (y._metadata ? y._metadata.modificationTime : 0);
> > 
> > Is modification time defined for directories?
> 
> Yes, directories have modification time. It'll be updated when its content is created or removed.
I think there two basic requirements:
 - directories should always precede files
 - "equal" entries should be sorted by name
You might want to use some file manager for the reference.

[Taiju Tsuiki]

Created attachment 150533 [details]
Patch

[Taiju Tsuiki]

Comment on attachment 150371 [details]
Patch

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

>>>> Source/WebCore/inspector/front-end/DirectoryContentView.js:139
>>>> +        if (x._entry.isDirectory)
>>> 
>>> This means directories order would be undefined. We should sort them by name instead.
>> 
>> I meant, all directories have same type.
>> Should we also sort other same-mimetype files by name?
> 
> I think there two basic requirements:
>  - directories should always precede files
>  - "equal" entries should be sorted by name
> You might want to use some file manager for the reference.

Updated.

The ordering is differ for each file manager. That looks ubuntu style.

[WebKit Review Bot]

Comment on attachment 150533 [details]
Patch

Clearing flags on attachment: 150533

Committed r121748: <http://trac.webkit.org/changeset/121748>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.