DirectoryContentView should have sortable columns.
Created attachment 150371 [details] Patch
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?
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.
(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.
Created attachment 150533 [details] Patch
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.
Comment on attachment 150533 [details] Patch Clearing flags on attachment: 150533 Committed r121748: <http://trac.webkit.org/changeset/121748>
All reviewed patches have been landed. Closing bug.