Last Comment Bug 670561 - show profile path in profile manager
: show profile path in profile manager
Status: RESOLVED FIXED
[good first bug]
:
Product: SeaMonkey
Classification: Client Software
Component: Startup & Profiles (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Edmund Wong (:ewong)
:
Mentors:
: 23260 25196 (view as bug list)
Depends on: 765643
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-10 12:12 PDT by Karsten Düsterloh
Modified: 2012-06-18 01:41 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Show profile path in profile manager. (4.71 KB, patch)
2011-07-17 00:10 PDT, Edmund Wong (:ewong)
no flags Details | Diff | Review
Show profile path in profile manager.(v2) (4.65 KB, patch)
2011-07-17 00:20 PDT, Edmund Wong (:ewong)
no flags Details | Diff | Review
Show profile path in profile manager.(v3) (5.03 KB, patch)
2011-07-17 07:44 PDT, Edmund Wong (:ewong)
mnyromyr: review-
bugspam.Callek: feedback+
Details | Diff | Review
Show profile path in profile manager. (v4) (3.72 KB, patch)
2011-11-05 07:22 PDT, Edmund Wong (:ewong)
mnyromyr: review+
Details | Diff | Review
Show profile path in profile manager. (v5) (3.09 KB, patch)
2011-11-16 07:45 PST, Edmund Wong (:ewong)
mnyromyr: review+
philip.chee: feedback+
Details | Diff | Review

Description Karsten Düsterloh 2011-07-10 12:12:04 PDT
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>
Comment 1 therube 2011-07-11 10:28:59 PDT
(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 :-)
Comment 2 Karsten Düsterloh 2011-07-11 23:02:52 PDT
This is pure SeaMonkey code, we don't have morons here. ;-)
Comment 3 Stefan [:stefanh] (away until May 28) 2011-07-12 12:00:23 PDT
Do we need to show it in prefs if we show it in profile manager?
Comment 4 Stefan [:stefanh] (away until May 28) 2011-07-12 12:24:36 PDT
(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...
Comment 5 Edmund Wong (:ewong) 2011-07-17 00:10:16 PDT
Created attachment 546393 [details] [diff] [review]
Show profile path in profile manager.
Comment 6 Edmund Wong (:ewong) 2011-07-17 00:20:38 PDT
Created attachment 546394 [details] [diff] [review]
Show profile path in profile manager.(v2)

Removed commented code.
Comment 7 Ian Neal 2011-07-17 03:10:52 PDT
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.
Comment 8 Edmund Wong (:ewong) 2011-07-17 07:44:22 PDT
Created attachment 546417 [details] [diff] [review]
Show profile path in profile manager.(v3)
Comment 9 Justin Wood (:Callek) 2011-08-06 05:01:43 PDT
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...
Comment 10 neil@parkwaycc.co.uk 2011-09-07 04:14:40 PDT
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.
Comment 11 Karsten Düsterloh 2011-09-11 14:14:08 PDT
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. ;-)
Comment 12 Edmund Wong (:ewong) 2011-11-05 07:22:51 PDT
Created attachment 572202 [details] [diff] [review]
Show profile path in profile manager. (v4)
Comment 13 Karsten Düsterloh 2011-11-06 10:18:57 PST
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.
Comment 14 Edmund Wong (:ewong) 2011-11-16 07:45:23 PST
Created attachment 574894 [details] [diff] [review]
Show profile path in profile manager. (v5)
Comment 15 Philip Chee 2011-11-16 12:00:59 PST
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.
Comment 16 Karsten Düsterloh 2011-11-25 09:05:17 PST
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. ;-)
Comment 17 Edmund Wong (:ewong) 2011-12-10 08:13:51 PST
Pushed to comm-central:

http://hg.mozilla.org/comm-central/rev/75669e449485
Comment 18 Philip Chee 2012-01-11 12:11:22 PST
*** Bug 23260 has been marked as a duplicate of this bug. ***
Comment 19 Philip Chee 2012-03-12 08:23:10 PDT
*** Bug 25196 has been marked as a duplicate of this bug. ***
Comment 20 neil@parkwaycc.co.uk 2012-06-17 17:05:31 PDT
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.

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