Closed
Bug 275223
Opened 20 years ago
Closed 12 years ago
Page Info columns should be sortable
Categories
(Firefox :: Page Info Window, enhancement)
Firefox
Page Info Window
Tracking
()
VERIFIED
FIXED
Firefox 12
People
(Reporter: mikronh, Assigned: felix)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
27.06 KB,
patch
|
db48x
:
review+
|
Details | Diff | Splinter Review |
While in Pag Info, in the "Media" tab. Pushing the "Address" or "Type" headlines does NOT sort by address/type.
Updated•20 years ago
|
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
Summary: No sorting in Page Info → Page Info columns should be sortable
Version: 1.0 Branch → unspecified
Comment 1•19 years ago
|
||
I thought this was a bug...Didn't it used to work in past versions?
Comment 2•19 years ago
|
||
*** Bug 317675 has been marked as a duplicate of this bug. ***
Comment 3•19 years ago
|
||
I don't recall this ever working. Also, the location of the horizontal sperator below the media list should be saved together with the window geometry.
Updated•17 years ago
|
Assignee: bugs → nobody
Updated•16 years ago
|
Version: unspecified → Trunk
Comment 9•14 years ago
|
||
> Tanner M. Young [:tmyoung] <mozilla.bugs@alyoung.com> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > See Also| |https://bugzilla.mozilla.or > | |g/show_bug.cgi?id=76170 The last time I put an internal b.m.o bug url in See Also it was moved to Blocks. I disagreed but was overruled.
Updated•14 years ago
|
Blocks: cuts-control
Comment 10•14 years ago
|
||
This simple thing, sorting by columns is very important. Trying to sort the media by type or by size in order to find a certain image for example is not possible now so you have to scroll over and over the long list ... It's time consuming, painful on the eyes and irritating. Please fix it soon. Thanks.
Assignee | ||
Comment 11•14 years ago
|
||
Attachment #488927 -
Flags: feedback?
Assignee | ||
Updated•14 years ago
|
Attachment #488927 -
Flags: feedback? → feedback?(johnath)
Comment 12•14 years ago
|
||
> - var sizeText;
> + var kbSize;
> if (cacheEntryDescriptor) {
> var pageSize = cacheEntryDescriptor.dataSize;
> - var kbSize = formatNumber(Math.round(pageSize / 1024 * 100) / 100);
> - sizeText = gBundle.getFormattedString("mediaFileSize", [kbSize]);
> + kbSize = Number(Math.round(pageSize / 1024 * 100) / 100);
> }
> else
> - sizeText = gStrings.unknown;
> - gImageView.addRow([url, type, sizeText, alt, 1, elem, isBg]);
> + kbSize = -1
> +
> + gImageView.addRow([url, type, kbSize, alt, 1, elem, isBg]);
Part of some other bug?
Assignee | ||
Comment 13•14 years ago
|
||
Summary of Changes - Moved permissionutils.js which contained a generic tree sort/delete functions to treeUtils.js in global - Changed treeSort function to do a check to see whether we want a lexicographical or numeric sort - gImageView was storing the size of the files as text as in "1.23 kB", so that wouldn't sort correctly. Changed it to store as floats and changed getCellValue to produce the correct output
Comment 15•14 years ago
|
||
Comment on attachment 488927 [details] [diff] [review] added non-persistent sorting to pageinfo, took tree utils from preferences >diff --git a/browser/base/content/pageinfo/pageInfo.js b/browser/base/content/pageinfo/pageInfo.js >--- a/browser/base/content/pageinfo/pageInfo.js >+++ b/browser/base/content/pageinfo/pageInfo.js >@@ -65,17 +65,25 @@ pageInfoTreeView.prototype = { > }, > > getCellText: function(row, column) > { > // row can be null, but js arrays are 0-indexed. > // colidx cannot be null, but can be larger than the number > // of columns in the array. In this case it's the fault of > // whoever typoed while calling this function. >- return this.data[row][column.index] || ""; >+ var text = this.data[row][column.index]; >+ if (column.index == COL_IMAGE_SIZE) { >+ if (text == -1) { >+ return gStrings.unknown; >+ } else { >+ return gBundle.getFormattedString("mediaFileSize", [text]); >+ } >+ } >+ return text || ""; > }, > > setCellValue: function(row, column, value) > { > }, > > setCellText: function(row, column, value) > { >@@ -580,25 +588,25 @@ function addImage(url, type, alt, elem, > catch(ex) { > try { > // open for READ, in non-blocking mode > cacheEntryDescriptor = ftpCacheSession.openCacheEntry(url, ACCESS_READ, false); > } > catch(ex2) { } > } > >- var sizeText; >+ var kbSize; > if (cacheEntryDescriptor) { > var pageSize = cacheEntryDescriptor.dataSize; >- var kbSize = formatNumber(Math.round(pageSize / 1024 * 100) / 100); >- sizeText = gBundle.getFormattedString("mediaFileSize", [kbSize]); >+ kbSize = Number(Math.round(pageSize / 1024 * 100) / 100); > } > else >- sizeText = gStrings.unknown; >- gImageView.addRow([url, type, sizeText, alt, 1, elem, isBg]); >+ kbSize = -1 >+ >+ gImageView.addRow([url, type, kbSize, alt, 1, elem, isBg]); > > // Add the observer, only once. > if (gImageView.data.length == 1) { > document.getElementById("mediaTab").hidden = false; > Components.classes["@mozilla.org/observer-service;1"] > .getService(Components.interfaces.nsIObserverService) > .addObserver(imagePermissionObserver, "perm-changed", false); > } >@@ -1236,8 +1244,31 @@ function selectImage() > function checkProtocol(img) > { > var url = img[COL_IMAGE_ADDRESS]; > if (/^data:/.test(url) && /^image\//.test(img[COL_IMAGE_NODE].type)) > return true; > const regex = /^(https?|ftp|file|about|chrome|resource):/; > return regex.test(url); > } >+ >+var gPageMediaInfo = { >+ _lastSortColumn : -1, >+ _lastSortAscending : false, >+ >+ onPageMediaSort : function (aColumn) >+ { >+ var tree = document.getElementById("imagetree"); >+ var column = tree.columns.getNamedColumn(aColumn); >+ >+ this._lastSortAscending = >+ gTreeUtils.sort( >+ tree, >+ gImageView, >+ gImageView.data, >+ column.index, >+ this._lastSortColumn, >+ this._lastSortAscending >+ ); >+ >+ this._lastSortColumn = column.index; >+ } >+}; >diff --git a/browser/base/content/pageinfo/pageInfo.xul b/browser/base/content/pageinfo/pageInfo.xul >--- a/browser/base/content/pageinfo/pageInfo.xul >+++ b/browser/base/content/pageinfo/pageInfo.xul >@@ -61,16 +61,17 @@ > onunload="onUnloadPageInfo()" > align="stretch" > screenX="10" screenY="10" > width="&pageInfoWindow.width;" height="&pageInfoWindow.height;" > persist="screenX screenY width height sizemode"> > > <script type="application/javascript" src="chrome://global/content/globalOverlay.js"/> > <script type="application/javascript" src="chrome://global/content/contentAreaUtils.js"/> >+ <script type="application/javascript" src="chrome://global/content/treeUtils.js"/> > <script type="application/javascript" src="chrome://browser/content/pageinfo/pageInfo.js"/> > <script type="application/javascript" src="chrome://browser/content/pageinfo/feeds.js"/> > <script type="application/javascript" src="chrome://browser/content/pageinfo/permissions.js"/> > <script type="application/javascript" src="chrome://browser/content/pageinfo/security.js"/> > <script type="application/javascript" src="chrome://browser/content/utilityOverlay.js"/> > > <stringbundleset id="pageinfobundleset"> > <stringbundle id="pageinfobundle" src="chrome://browser/locale/pageInfo.properties"/> >@@ -213,29 +214,34 @@ > </vbox> > > <!-- Media information --> > <vbox id="mediaPanel"> > <tree id="imagetree" onselect="onImageSelect();" contextmenu="picontext" > ondragstart="onBeginLinkDrag(event,'image-address','image-alt')"> > <treecols> > <treecol sortSeparators="true" primary="true" persist="width" flex="10" >- width="10" id="image-address" label="&mediaAddress;"/> >+ width="10" id="image-address" label="&mediaAddress;" >+ onclick="gPageMediaInfo.onPageMediaSort('image-address');"/> > <splitter class="tree-splitter"/> > <treecol sortSeparators="true" persist="hidden width" flex="2" >- width="2" id="image-type" label="&mediaType;"/> >+ width="2" id="image-type" label="&mediaType;" >+ onclick="gPageMediaInfo.onPageMediaSort('image-type');"/> > <splitter class="tree-splitter"/> > <treecol sortSeparators="true" hidden="true" persist="hidden width" flex="2" >- width="2" id="image-size" label="&mediaSize;"/> >+ width="2" id="image-size" label="&mediaSize;" value="size" >+ onclick="gPageMediaInfo.onPageMediaSort('image-size');"/> > <splitter class="tree-splitter"/> > <treecol sortSeparators="true" hidden="true" persist="hidden width" flex="4" >- width="4" id="image-alt" label="&mediaAltHeader;"/> >+ width="4" id="image-alt" label="&mediaAltHeader;" >+ onclick="gPageMediaInfo.onPageMediaSort('image-alt');"/> > <splitter class="tree-splitter"/> > <treecol sortSeparators="true" hidden="true" persist="hidden width" flex="1" >- width="1" id="image-count" label="&mediaCount;"/> >+ width="1" id="image-count" label="&mediaCount;" >+ onclick="gPageMediaInfo.onPageMediaSort('image-count');"/> > </treecols> > <treechildren flex="1"/> > </tree> > <splitter orient="vertical" id="mediaSplitter"/> > <vbox flex="1" id="mediaPreviewBox" collapsed="true"> > <grid id="mediaGrid"> > <columns> > <column id="mediaLabelColumn"/> >diff --git a/browser/components/preferences/cookies.xul b/browser/components/preferences/cookies.xul >--- a/browser/components/preferences/cookies.xul >+++ b/browser/components/preferences/cookies.xul >@@ -46,17 +46,17 @@ > <window id="CookiesDialog" windowtype="Browser:Cookies" > class="windowDialog" title="&window.title;" > xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" > style="width: &window.width;;" > onload="gCookiesWindow.init();" > onunload="gCookiesWindow.uninit();" > persist="screenX screenY width height"> > >- <script src="chrome://browser/content/preferences/permissionsutils.js"/> >+ <script src="chrome://global/content/treeUtils.js"/> > <script src="chrome://browser/content/preferences/cookies.js"/> > > <stringbundle id="bundlePreferences" > src="chrome://browser/locale/preferences/preferences.properties"/> > > <keyset> > <key key="&windowClose.key;" modifiers="accel" oncommand="window.close();"/> > <key key="&focusSearch1.key;" modifiers="accel" oncommand="gCookiesWindow.focusFilterBox();"/> >diff --git a/browser/components/preferences/jar.mn b/browser/components/preferences/jar.mn >--- a/browser/components/preferences/jar.mn >+++ b/browser/components/preferences/jar.mn >@@ -18,17 +18,16 @@ browser.jar: > * content/browser/preferences/handlers.xml > * content/browser/preferences/handlers.css > * content/browser/preferences/languages.xul > * content/browser/preferences/languages.js > * content/browser/preferences/main.xul > * content/browser/preferences/main.js > * content/browser/preferences/permissions.xul > * content/browser/preferences/permissions.js >-* content/browser/preferences/permissionsutils.js > * content/browser/preferences/preferences.xul > * content/browser/preferences/privacy.xul > * content/browser/preferences/privacy.js > * content/browser/preferences/sanitize.xul > * content/browser/preferences/security.xul > * content/browser/preferences/security.js > * content/browser/preferences/selectBookmark.xul > * content/browser/preferences/selectBookmark.js >diff --git a/browser/components/preferences/permissions.xul b/browser/components/preferences/permissions.xul >--- a/browser/components/preferences/permissions.xul >+++ b/browser/components/preferences/permissions.xul >@@ -48,17 +48,17 @@ > windowtype="Browser:Permissions" > title="&window.title;" > xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" > style="width: &window.width;;" > onload="gPermissionManager.onLoad();" > onunload="gPermissionManager.uninit();" > persist="screenX screenY width height"> > >- <script src="chrome://browser/content/preferences/permissionsutils.js"/> >+ <script src="chrome://global/content/treeUtils.js"/> > <script src="chrome://browser/content/preferences/permissions.js"/> > > <stringbundle id="bundlePreferences" > src="chrome://browser/locale/preferences/preferences.properties"/> > > <keyset> > <key key="&windowClose.key;" modifiers="accel" oncommand="window.close();"/> > </keyset> >diff --git a/browser/components/preferences/permissionsutils.js b/browser/components/preferences/permissionsutils.js >deleted file mode 100644 >--- a/browser/components/preferences/permissionsutils.js >+++ /dev/null >@@ -1,102 +0,0 @@ >-# -*- Mode: Java; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- >-# ***** BEGIN LICENSE BLOCK ***** >-# Version: MPL 1.1/GPL 2.0/LGPL 2.1 >-# >-# The contents of this file are subject to the Mozilla Public License Version >-# 1.1 (the "License"); you may not use this file except in compliance with >-# the License. You may obtain a copy of the License at >-# http://www.mozilla.org/MPL/ >-# >-# Software distributed under the License is distributed on an "AS IS" basis, >-# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License >-# for the specific language governing rights and limitations under the >-# License. >-# >-# The Original Code is the Firefox Preferences System. >-# >-# The Initial Developer of the Original Code is >-# Ben Goodger. >-# Portions created by the Initial Developer are Copyright (C) 2005 >-# the Initial Developer. All Rights Reserved. >-# >-# Contributor(s): >-# Ben Goodger <ben@mozilla.org> >-# >-# Alternatively, the contents of this file may be used under the terms of >-# either the GNU General Public License Version 2 or later (the "GPL"), or >-# the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), >-# in which case the provisions of the GPL or the LGPL are applicable instead >-# of those above. If you wish to allow use of your version of this file only >-# under the terms of either the GPL or the LGPL, and not to allow others to >-# use your version of this file under the terms of the MPL, indicate your >-# decision by deleting the provisions above and replace them with the notice >-# and other provisions required by the GPL or the LGPL. If you do not delete >-# the provisions above, a recipient may use your version of this file under >-# the terms of any one of the MPL, the GPL or the LGPL. >-# >-# ***** END LICENSE BLOCK ***** >- >-var gTreeUtils = { >- deleteAll: function (aTree, aView, aItems, aDeletedItems) >- { >- for (var i = 0; i < aItems.length; ++i) >- aDeletedItems.push(aItems[i]); >- aItems.splice(0); >- var oldCount = aView.rowCount; >- aView._rowCount = 0; >- aTree.treeBoxObject.rowCountChanged(0, -oldCount); >- }, >- >- deleteSelectedItems: function (aTree, aView, aItems, aDeletedItems) >- { >- var selection = aTree.view.selection; >- selection.selectEventsSuppressed = true; >- >- var rc = selection.getRangeCount(); >- for (var i = 0; i < rc; ++i) { >- var min = { }; var max = { }; >- selection.getRangeAt(i, min, max); >- for (var j = min.value; j <= max.value; ++j) { >- aDeletedItems.push(aItems[j]); >- aItems[j] = null; >- } >- } >- >- var nextSelection = 0; >- for (i = 0; i < aItems.length; ++i) { >- if (!aItems[i]) { >- var j = i; >- while (j < aItems.length && !aItems[j]) >- ++j; >- aItems.splice(i, j - i); >- nextSelection = j < aView.rowCount ? j - 1 : j - 2; >- aView._rowCount -= j - i; >- aTree.treeBoxObject.rowCountChanged(i, i - j); >- } >- } >- >- if (aItems.length) { >- selection.select(nextSelection); >- aTree.treeBoxObject.ensureRowIsVisible(nextSelection); >- aTree.focus(); >- } >- selection.selectEventsSuppressed = false; >- }, >- >- sort: function (aTree, aView, aDataSet, aColumn, >- aLastSortColumn, aLastSortAscending) >- { >- var ascending = (aColumn == aLastSortColumn) ? !aLastSortAscending : true; >- aDataSet.sort(function (a, b) { return a[aColumn].toLowerCase().localeCompare(b[aColumn].toLowerCase()); }); >- if (!ascending) >- aDataSet.reverse(); >- >- aTree.view.selection.select(-1); >- aTree.view.selection.select(0); >- aTree.treeBoxObject.invalidate(); >- aTree.treeBoxObject.ensureRowIsVisible(0); >- >- return ascending; >- } >-}; >- >diff --git a/toolkit/content/jar.mn b/toolkit/content/jar.mn >--- a/toolkit/content/jar.mn >+++ b/toolkit/content/jar.mn >@@ -32,16 +32,17 @@ toolkit.jar: > *+ content/global/editMenuOverlay.xul (editMenuOverlay.xul) > *+ content/global/finddialog.js (finddialog.js) > *+ content/global/finddialog.xul (finddialog.xul) > *+ content/global/findUtils.js (findUtils.js) > content/global/filepicker.properties (filepicker.properties) > *+ content/global/globalOverlay.js (globalOverlay.js) > + content/global/mozilla.xhtml (mozilla.xhtml) > *+ content/global/nsDragAndDrop.js (nsDragAndDrop.js) >+* content/global/treeUtils.js (treeUtils.js) > *+ content/global/viewZoomOverlay.js (viewZoomOverlay.js) > *+ content/global/bindings/autocomplete.xml (widgets/autocomplete.xml) > *+ content/global/bindings/browser.xml (widgets/browser.xml) > *+ content/global/bindings/button.xml (widgets/button.xml) > *+ content/global/bindings/checkbox.xml (widgets/checkbox.xml) > *+ content/global/bindings/colorpicker.xml (widgets/colorpicker.xml) > *+ content/global/bindings/datetimepicker.xml (widgets/datetimepicker.xml) > *+ content/global/bindings/dialog.xml (widgets/dialog.xml) >diff --git a/toolkit/content/treeUtils.js b/toolkit/content/treeUtils.js >new file mode 100644 >--- /dev/null >+++ b/toolkit/content/treeUtils.js >@@ -0,0 +1,112 @@ >+# -*- Mode: Java; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- >+# ***** BEGIN LICENSE BLOCK ***** >+# Version: MPL 1.1/GPL 2.0/LGPL 2.1 >+# >+# The contents of this file are subject to the Mozilla Public License Version >+# 1.1 (the "License"); you may not use this file except in compliance with >+# the License. You may obtain a copy of the License at >+# http://www.mozilla.org/MPL/ >+# >+# Software distributed under the License is distributed on an "AS IS" basis, >+# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License >+# for the specific language governing rights and limitations under the >+# License. >+# >+# The Original Code is the Firefox Preferences System. >+# >+# The Initial Developer of the Original Code is >+# Ben Goodger. >+# Portions created by the Initial Developer are Copyright (C) 2005 >+# the Initial Developer. All Rights Reserved. >+# >+# Contributor(s): >+# Ben Goodger <ben@mozilla.org> >+# >+# Alternatively, the contents of this file may be used under the terms of >+# either the GNU General Public License Version 2 or later (the "GPL"), or >+# the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), >+# in which case the provisions of the GPL or the LGPL are applicable instead >+# of those above. If you wish to allow use of your version of this file only >+# under the terms of either the GPL or the LGPL, and not to allow others to >+# use your version of this file under the terms of the MPL, indicate your >+# decision by deleting the provisions above and replace them with the notice >+# and other provisions required by the GPL or the LGPL. If you do not delete >+# the provisions above, a recipient may use your version of this file under >+# the terms of any one of the MPL, the GPL or the LGPL. >+# >+# ***** END LICENSE BLOCK ***** >+ >+var gTreeUtils = { >+ deleteAll: function (aTree, aView, aItems, aDeletedItems) >+ { >+ for (var i = 0; i < aItems.length; ++i) >+ aDeletedItems.push(aItems[i]); >+ aItems.splice(0); >+ var oldCount = aView.rowCount; >+ aView._rowCount = 0; >+ aTree.treeBoxObject.rowCountChanged(0, -oldCount); >+ }, >+ >+ deleteSelectedItems: function (aTree, aView, aItems, aDeletedItems) >+ { >+ var selection = aTree.view.selection; >+ selection.selectEventsSuppressed = true; >+ >+ var rc = selection.getRangeCount(); >+ for (var i = 0; i < rc; ++i) { >+ var min = { }; var max = { }; >+ selection.getRangeAt(i, min, max); >+ for (var j = min.value; j <= max.value; ++j) { >+ aDeletedItems.push(aItems[j]); >+ aItems[j] = null; >+ } >+ } >+ >+ var nextSelection = 0; >+ for (i = 0; i < aItems.length; ++i) { >+ if (!aItems[i]) { >+ var j = i; >+ while (j < aItems.length && !aItems[j]) >+ ++j; >+ aItems.splice(i, j - i); >+ nextSelection = j < aView.rowCount ? j - 1 : j - 2; >+ aView._rowCount -= j - i; >+ aTree.treeBoxObject.rowCountChanged(i, i - j); >+ } >+ } >+ >+ if (aItems.length) { >+ selection.select(nextSelection); >+ aTree.treeBoxObject.ensureRowIsVisible(nextSelection); >+ aTree.focus(); >+ } >+ selection.selectEventsSuppressed = false; >+ }, >+ >+ sort: function (aTree, aView, aDataSet, aColumn, >+ aLastSortColumn, aLastSortAscending) >+ { >+ var ascending = (aColumn == aLastSortColumn) ? !aLastSortAscending : true; >+ if (aDataSet.length == 0) >+ return ascending; >+ >+ var numericSort = !isNaN(aDataSet[0][aColumn]); >+ var sortFunction; >+ if (numericSort) { >+ sortFunction = function (a, b) { return a[aColumn] - b[aColumn]; } >+ } else { >+ sortFunction = function (a, b) { return a[aColumn].toLowerCase().localeCompare(b[aColumn].toLowerCase()); } >+ } >+ aDataSet.sort(sortFunction); >+ >+ if (!ascending) >+ aDataSet.reverse(); >+ >+ aTree.view.selection.select(0); >+ aTree.treeBoxObject.invalidate(); >+ aTree.treeBoxObject.ensureRowIsVisible(0); >+ >+ return ascending; >+ } >+}; >+
Attachment #488927 -
Flags: review?(db48x)
Comment 16•14 years ago
|
||
Comment on attachment 488927 [details] [diff] [review] added non-persistent sorting to pageinfo, took tree utils from preferences First, all of the trees in the Page Info dialog box should be sortable, not just the one on the media tab. >diff --git a/browser/base/content/pageinfo/pageInfo.js b/browser/base/content/pageinfo/pageInfo.js >--- a/browser/base/content/pageinfo/pageInfo.js >+++ b/browser/base/content/pageinfo/pageInfo.js >@@ -65,17 +65,25 @@ pageInfoTreeView.prototype = { > }, > > getCellText: function(row, column) > { > // row can be null, but js arrays are 0-indexed. > // colidx cannot be null, but can be larger than the number > // of columns in the array. In this case it's the fault of > // whoever typoed while calling this function. >- return this.data[row][column.index] || ""; >+ var text = this.data[row][column.index]; >+ if (column.index == COL_IMAGE_SIZE) { >+ if (text == -1) { >+ return gStrings.unknown; >+ } else { >+ return gBundle.getFormattedString("mediaFileSize", [text]); >+ } >+ } >+ return text || ""; > }, I don't really like this. For one thing, this will apply to all the trees in the window if they have enough columns. Create a subclass that overrides this method and use it just for the media tree. >+var gPageMediaInfo = { >+ _lastSortColumn : -1, >+ _lastSortAscending : false, >+ >+ onPageMediaSort : function (aColumn) >+ { >+ var tree = document.getElementById("imagetree"); >+ var column = tree.columns.getNamedColumn(aColumn); >+ >+ this._lastSortAscending = >+ gTreeUtils.sort( >+ tree, >+ gImageView, >+ gImageView.data, >+ column.index, >+ this._lastSortColumn, >+ this._lastSortAscending >+ ); >+ >+ this._lastSortColumn = column.index; >+ } >+}; Store this information on the individual treeview instances, so that we don't have a lot of different global objects floating around. This is looking pretty good though; thanks for helping :)
Attachment #488927 -
Flags: review?(db48x) → review-
Assignee | ||
Comment 18•13 years ago
|
||
New Changes As Per db48x's Requests: - getCellText is subclassed by gImageView rather than modifying all pageInfoTreeViews - moved sorting state and onPageMediaSort into pageInfoTreeViews rather than craete new global variables Summary of Changes - Moved permissionutils.js which contained a generic tree sort/delete functions to treeUtils.js in global - Changed treeSort function to do a check to see whether we want a lexicographical or numeric sort - gImageView was storing the size of the files as text as in "1.23 kB", so that wouldn't sort correctly. Changed it to store as floats and changed getCellValue to produce the correct output
Attachment #488927 -
Attachment is obsolete: true
Attachment #558744 -
Flags: review?(db48x)
Attachment #488927 -
Flags: feedback?(johnath)
Updated•13 years ago
|
Assignee: nobody → ffung
Comment 19•12 years ago
|
||
Comment on attachment 558744 [details] [diff] [review] Give all tree views in page info sortable columns Review of attachment 558744 [details] [diff] [review]: ----------------------------------------------------------------- I have been remiss! Felix, this looks like a great patch. I would like you to make one small change, however. ::: browser/base/content/pageinfo/pageInfo.js @@ +205,5 @@ > props.AppendElement(this._ltrAtom); > }; > > +gImageView.getCellText = function(row, column) { > + var text = this.data[row][column.index]; Might as well call this variable size instead of text. @@ +619,3 @@ > if (cacheEntryDescriptor) { > var pageSize = cacheEntryDescriptor.dataSize; > + kbSize = Number(Math.round(pageSize / 1024 * 100) / 100); It would be better to store the actual size in the tree to sort by, then do the division in gImageView.getCellText above. This puts all of the presentation logic in one place, making it easier to extend that logic in the future.
Attachment #558744 -
Flags: review?(db48x) → review-
Assignee | ||
Comment 20•12 years ago
|
||
Additional Changes: - Changed signature to gTreeUtils.sort to take a comparator - Renamed variable 'text' to 'value' as per comment above. (Size isn't quite right either since it can be a value from any column) - Moved KB display logic to getCellText to gather all the presentation logic
Attachment #558744 -
Attachment is obsolete: true
Attachment #591384 -
Flags: review?(db48x)
Comment 21•12 years ago
|
||
Comment on attachment 591384 [details] [diff] [review] Give all tree views in page info sortable columns Review of attachment 591384 [details] [diff] [review]: ----------------------------------------------------------------- great. r=db48x
Attachment #591384 -
Flags: review?(db48x) → review+
Assignee | ||
Comment 22•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ffb30085569
Comment 23•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3ffb30085569
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
Comment 24•12 years ago
|
||
Comment on attachment 591384 [details] [diff] [review] >@@ -116,16 +117,35 @@ pageInfoTreeView.prototype = { > performActionOnRow: function(action, row) > { > if (action == "copy") { > var data = this.handleCopy(row) > this.tree.treeBody.parentNode.setAttribute("copybuffer", data); > } > }, > >+ onPageMediaSort : function(columnname) A comment: The result of this copy and paste is probably not what you wanted. pageInfoTreeView.prototype now has an onPageMediaSort method for sorting the Meta tree, which is on the General page, not the Media page. For future reference: > <treecol id="meta-name" label="&generalMetaName;" >- persist="width" flex="1"/> >+ persist="width" flex="1" >+ onclick="gMetaView.onPageMediaSort('meta-name');"/> > <splitter class="tree-splitter"/> > <treecol id="meta-content" label="&generalMetaContent;" >- persist="width" flex="4"/> >+ persist="width" flex="4" >+ onclick="gMetaView.onPageMediaSort('meta-content');"/> > <treecol sortSeparators="true" primary="true" persist="width" flex="10" >- width="10" id="image-address" label="&mediaAddress;"/> >+ width="10" id="image-address" label="&mediaAddress;" >+ onclick="gImageView.onPageMediaSort('image-address');"/> > <splitter class="tree-splitter"/> > <treecol sortSeparators="true" persist="hidden width" flex="2" >- width="2" id="image-type" label="&mediaType;"/> >+ width="2" id="image-type" label="&mediaType;" >+ onclick="gImageView.onPageMediaSort('image-type');"/> > <splitter class="tree-splitter"/> > <treecol sortSeparators="true" hidden="true" persist="hidden width" flex="2" >- width="2" id="image-size" label="&mediaSize;"/> >+ width="2" id="image-size" label="&mediaSize;" value="size" >+ onclick="gImageView.onPageMediaSort('image-size');"/> > <splitter class="tree-splitter"/> > <treecol sortSeparators="true" hidden="true" persist="hidden width" flex="4" >- width="4" id="image-alt" label="&mediaAltHeader;"/> >+ width="4" id="image-alt" label="&mediaAltHeader;" >+ onclick="gImageView.onPageMediaSort('image-alt');"/> > <splitter class="tree-splitter"/> > <treecol sortSeparators="true" hidden="true" persist="hidden width" flex="1" >- width="1" id="image-count" label="&mediaCount;"/> >+ width="1" id="image-count" label="&mediaCount;" >+ onclick="gImageView.onPageMediaSort('image-count');"/> What you would've wanted to do here is just implement pageInfoTreeView.prototype.cycleHeader and override it for gImageView, instead of adding extra click handlers. >-function pageInfoTreeView(copycol) >+function pageInfoTreeView(treeid, copycol) > { > // copycol is the index number for the column that we want to add to > // the copy-n-paste buffer when the user hits accel-c >+ this.treeid = treeid; this.tree.element in the only consumer (onPageMediaSort/cycleHeader) would've given you what you needed. It also wouldn't have required changing the constructor callsites: >-var gMetaView = new pageInfoTreeView(COPYCOL_META_CONTENT); >-var gImageView = new pageInfoTreeView(COPYCOL_IMAGE); >- >+var gMetaView = new pageInfoTreeView('metatree', COPYCOL_META_CONTENT); >+var gImageView = new pageInfoTreeView('imagetree', COPYCOL_IMAGE);
Target Milestone: Firefox 12 → ---
Version: Trunk → 13 Branch
Comment 25•12 years ago
|
||
Don't know why those fields changed...
Target Milestone: --- → Firefox 12
Version: 13 Branch → Trunk
Assignee | ||
Comment 26•12 years ago
|
||
>The result of this copy and paste is probably not what you wanted. >pageInfoTreeView.prototype now has an onPageMediaSort method for sorting the Meta >tree, which is on the General page, not the Media page. See comment 16, I intended the meta tree to be sortable. I will look into overriding cycleHeader
Comment 27•12 years ago
|
||
(In reply to Felix Fung (:felix) from comment #26) > >The result of this copy and paste is probably not what you wanted. >pageInfoTreeView.prototype now has an onPageMediaSort method for sorting the Meta >tree, which is on the General page, not the Media page. > > See comment 16, I intended the meta tree to be sortable. Right. You probably didn't want the function to be called onPageMediaSort, though, since the Meta tree isn't on the Media page. :) By the way, thanks for the patch.
Comment 28•12 years ago
|
||
It seems that you guys work hard for this feature. I admit I do not have the ability to understand your code, but still, I would like to ask why is it so complicated to sort items (files sizes in this case)? Will I be able to enjoy this feature during my life time?
Comment 29•12 years ago
|
||
(In reply to t8av from comment #28) > Will I be able to enjoy this feature during my life time? This bug is fixed in Firefox 12, which will be released in April. You can download a pre-release version today from the "Aurora" channel: http://mozilla.org/firefox/aurora/
Comment 30•12 years ago
|
||
> I will look into overriding cycleHeader I used cycleHeader in the SeaMonkey version of this bug in Bug 76170. I also fixed a bug exposed by making the image tree columns sortable. This bug also exists in Firefox.
Comment 31•12 years ago
|
||
(In reply to Matt Brubeck (:mbrubeck) from comment #29) > (In reply to t8av from comment #28) > > Will I be able to enjoy this feature during my life time? > > This bug is fixed in Firefox 12, which will be released in April. You can > download a pre-release version today from the "Aurora" channel: > > http://mozilla.org/firefox/aurora/ Sorry, maybe I am blind, but now it is already Firefox version 15, and I still cannot sort the items by size.
Comment 32•12 years ago
|
||
Works fine for me with the tested Firefox 14.0 version. You have to click the column header to sort by type in ascending/descending mode.
Status: RESOLVED → VERIFIED
Comment 33•12 years ago
|
||
Yes, now I can see it. The field "Size" should be shown by default - please do not expect the user to find it by himself in the right icon.
You need to log in
before you can comment on or make changes to this bug.
Description
•