Closed
Bug 670561
Opened 13 years ago
Closed 13 years ago
show profile path in profile manager
Categories
(SeaMonkey :: Startup & Profiles, defect)
SeaMonkey
Startup & Profiles
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mnyromyr, Assigned: ewong)
References
Details
(Whiteboard: [good first bug])
Attachments
(1 file, 4 obsolete files)
3.09 KB,
patch
|
mnyromyr
:
review+
philip.chee
:
feedback+
|
Details | Diff | Splinter Review |
Show profile path in profile manager when hovering over a listbox entry like toolkit does: see <http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/profile/content/profileSelection.js#79>
(Perhaps you'll be shot down for similar reasons, Bug 658186 - RFE: about:support Provide "Profile Directory" Path/Name in addition to Open Button ?) But considering you're only looking for a mouseover. I like the idea :-)
Reporter | ||
Comment 2•13 years ago
|
||
This is pure SeaMonkey code, we don't have morons here. ;-)
Comment 3•13 years ago
|
||
Do we need to show it in prefs if we show it in profile manager?
Comment 4•13 years ago
|
||
(In reply to comment #3) > Do we need to show it in prefs if we show it in profile manager? Erm, forget what I said... I'm trying to honour "Think before you type", but here I failed...
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•13 years ago
|
||
Attachment #546393 -
Flags: feedback?(bugspam.Callek)
Assignee | ||
Comment 6•13 years ago
|
||
Removed commented code.
Attachment #546393 -
Attachment is obsolete: true
Attachment #546394 -
Flags: feedback?(bugspam.Callek)
Attachment #546393 -
Flags: feedback?(bugspam.Callek)
Comment on attachment 546394 [details] [diff] [review] Show profile path in profile manager.(v2) >+++ b/suite/common/profile/profileSelection.js > var selectedProfile = null; >+ var profileName = ''; This should probably be called profile and no need to set to = '' >+ var profileDir = ''; No need to set to = '' >+ var profileInfo = new Object(); I don't think we need to complicate things by having this variable. > while (profileEnum.hasMoreElements()) { >- AddItem(profileEnum.getNext().QueryInterface(Components.interfaces.nsIToolkitProfile), >- selectedProfile); >+ profileName = profileEnum.getNext().QueryInterface(Components.interfaces.nsIToolkitProfile); From above, just call this variable profile. >+ profileDir = gProfileService.getProfileByName(profileName.name).localDir; >+ profileInfo.profile = profileName; >+ profileInfo.profileDir = profileDir; >+ AddItem(profileInfo, selectedProfile); Just pass both profile and profileDir as arguments instead of creating the object. > function AddItem(aProfile, aProfileToSelect) So this would have aProfile, aProfileDir and aProfileToSelect. > { > var tree = document.getElementById("profiles"); > var treeitem = document.createElement("treeitem"); > var treerow = document.createElement("treerow"); > var treecell = document.createElement("treecell"); >- treecell.setAttribute("label", aProfile.name); >+ treecell.setAttribute("label", aProfile.profile.name); This line would no longer have to be changed. >+ treeitem.setAttribute("tooltip", aProfile.profileDir.persistentDescriptor); This would have to be changed still but using aProfileDir instead. > treerow.appendChild(treecell); > treeitem.appendChild(treerow); > tree.lastChild.appendChild(treeitem); >- treeitem.profile = aProfile; >- if (aProfile == aProfileToSelect) { >+ treeitem.profile = aProfile.profile; >+ if (aProfile.profile == aProfileToSelect) { These two lines would no longer have to be changed.
Assignee | ||
Comment 8•13 years ago
|
||
Attachment #546394 -
Attachment is obsolete: true
Attachment #546417 -
Flags: feedback?(bugspam.Callek)
Attachment #546394 -
Flags: feedback?(bugspam.Callek)
Comment 9•13 years ago
|
||
Comment on attachment 546417 [details] [diff] [review] Show profile path in profile manager.(v3) Looks like a good approach to me, you'll want review from someone more familiar with this code though...
Attachment #546417 -
Flags: feedback?(bugspam.Callek) → feedback+
Assignee | ||
Updated•13 years ago
|
Attachment #546417 -
Flags: review?(mnyromyr)
Comment 10•13 years ago
|
||
Comment on attachment 546417 [details] [diff] [review] Show profile path in profile manager.(v3) >+ profileDir = gProfileService.getProfileByName(profile.name).localDir; This should be the .rootDir, which is the main profile directory; the .localDir is only used for the various caches. >+ treeitem.setAttribute("tooltip", aProfileDir.persistentDescriptor); persistentDescriptor is an internal method used for localised file preferences. Its value should not be used in the UI! You probably want to use .path instead. >+function HandlePopEvent(aEvent) [Scope for bikeshedding] >+ var toolTip = aEvent.target.triggerNode.children.item(row.value).tooltip; tree.view.getItemAtIndex(row.value).tooltip >+ treeTip.firstChild.value = toolTip; Just set the .label directly on the tooltip itself. See below. >+ <tooltip id="treetip" >+ onpopupshowing="HandlePopEvent(event);"> >+ <label value=""/> tooltips already contain a label by default; only people who need something more sophisticated need to add their own children.
Reporter | ||
Comment 11•13 years ago
|
||
Comment on attachment 546417 [details] [diff] [review] Show profile path in profile manager.(v3) Nothing serious, just some nits: >+ var profile; >+ var profileDir; I don't see the point of these two variables, because … >- AddItem(profileEnum.getNext().QueryInterface(Components.interfaces.nsIToolkitProfile), >- selectedProfile); >+ profile = profileEnum.getNext().QueryInterface(Components.interfaces.nsIToolkitProfile); >+ profileDir = gProfileService.getProfileByName(profile.name).localDir; >+ AddItem(profile, profileDir, selectedProfile); … calculating profileDir here just to pass it down to AddItem is nonsense. Just extract the dir inside AddItem. >+function AddItem(aProfile, aProfileDir, aProfileToSelect) No need to change the function signature, see above. >+function HandlePopEvent(aEvent) >+{ >+ var treeTip = document.getElementById("treetip"); >+ var tree = document.getElementById("profiles"); >+ var elt = {}; >+ var row = {}; >+ var col = {}; >+ >+ tree.treeBoxObject.getCellAt(aEvent.clientX, aEvent.clientY, row, col, elt); You can replace col and elt by using {} in the call directly, since you don't use them anyway. >+ var toolTip = aEvent.target.triggerNode.children.item(row.value).tooltip; >+ treeTip.firstChild.value = toolTip; Permission granted to drop the toolTip variable without line break. ;-)
Attachment #546417 -
Flags: review?(mnyromyr) → review-
Assignee | ||
Comment 12•13 years ago
|
||
Attachment #546417 -
Attachment is obsolete: true
Attachment #572202 -
Flags: review?(mnyromyr)
Reporter | ||
Comment 13•13 years ago
|
||
Comment on attachment 572202 [details] [diff] [review] Show profile path in profile manager. (v4) >+++ b/suite/common/profile/profileSelection.js … > function AddItem(aProfile, aProfileToSelect) … function CreateProfile(aProfile) > { > gProfileService.flush(); >- AddItem(aProfile, aProfile); >+ AddItem(aProfile, aProfile, aProfile); > } This change is not needed anymore, the third parameter is ignored anyway. r=me with that.
Attachment #572202 -
Flags: review?(mnyromyr) → review+
Assignee | ||
Comment 14•13 years ago
|
||
Attachment #572202 -
Attachment is obsolete: true
Attachment #574894 -
Flags: feedback?(philip.chee)
Comment 15•13 years ago
|
||
Comment on attachment 574894 [details] [diff] [review] Show profile path in profile manager. (v5) Patch works. I see tooltips showing the path when hovering over individual profiles.
Attachment #574894 -
Flags: feedback?(philip.chee) → feedback+
Assignee | ||
Updated•13 years ago
|
Attachment #574894 -
Flags: review?(mnyromyr)
Reporter | ||
Comment 16•13 years ago
|
||
Comment on attachment 574894 [details] [diff] [review] Show profile path in profile manager. (v5) (In reply to Karsten Düsterloh from comment #13) > r=me with that. This meant to mean „if you do (just) those changes, you don't need to ask me again, just set r+ yourself and go on“. Of course, only if you feel comfortable with that. ;-)
Attachment #574894 -
Flags: review?(mnyromyr) → review+
Assignee | ||
Comment 17•13 years ago
|
||
Pushed to comm-central: http://hg.mozilla.org/comm-central/rev/75669e449485
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 20•12 years ago
|
||
Sadly this has a bug; if you hover over a blank area of the tree you get a blank tooltip, and additionally an assertion in debug builds.
You need to log in
before you can comment on or make changes to this bug.
Description
•