Closed Bug 545217 Opened 10 years ago Closed 10 years ago

rename "Smart folders" to "Unified folders"

Categories

(Thunderbird :: Folder and Message Lists, defect)

defect
Not set

Tracking

(blocking-thunderbird3.1 beta2+, thunderbird3.1 beta2-fixed)

RESOLVED FIXED
Thunderbird 3.1b2
Tracking Status
blocking-thunderbird3.1 --- beta2+
thunderbird3.1 --- beta2-fixed

People

(Reporter: clarkbw, Assigned: clarkbw)

References

Details

(Whiteboard: [UXprio])

Attachments

(1 file, 4 obsolete files)

A number of points of feedback have shown that the term "unified" is clearer than the term "smart" for the smart folders system.

Lets change the string, even though it appears to be more difficult than that.
Also, while glancing at the code I noticed this variable seems to be unused.

diff --git a/mail/base/content/folderPane.js b/mail/base/content/folderPane.js
--- a/mail/base/content/folderPane.js
+++ b/mail/base/content/folderPane.js
@@ -136,19 +136,16 @@ let gFolderTreeView = {
    * Called when the window is initially loaded.  This function initializes the
    * folder-pane to the view last shown before the application was closed.
    */
   load: function ftv_load(aTree, aJSONFile) {
     const Cc = Components.classes;
     const Ci = Components.interfaces;
     this._treeElement = aTree;
 
-    let smartName = document.getElementById("bundle_messenger")
-                            .getString("folderPaneHeader_smart");
-
     // the folder pane can be used for other trees which may not have these elements.
     if (document.getElementById("folderpane_splitter"))
       document.getElementById("folderpane_splitter").collapsed = false;
     if (document.getElementById("folderPaneBox"))
       document.getElementById("folderPaneBox").collapsed = false;
 
     try {
       // Normally our tree takes care of keeping the last selected by itself.
Can we mark these blocking ? Would be nice to have them in b1 to gather feedback.
Whiteboard: [UXprio]
blocking-thunderbird3.1: --- → beta1+
Whiteboard: [UXprio] → [UXprio][needs patch]
Assignee: nobody → clarkbw
Whiteboard: [UXprio][needs patch] → [UXprio][needs patch clarkbw, review, landing]
Simon - 

Because of how the code is structured it seems like we have to change all the folder pane header strings as there is no easy way to just change the smart folders header string.

http://mxr.mozilla.org/comm-central/search?string=folderPaneHeader_

We can land this change now, but if you feel it's too late we can wait until after the b1 release as this isn't a critical change which requires a lot of testing.

I'll be attaching the patch shortly.
(In reply to comment #1)
> Also, while glancing at the code I noticed this variable seems to be unused.

NOTE: This change was handled in attachment 428478 [details] [diff] [review] of bug 545221
Whiteboard: [UXprio][needs patch clarkbw, review, landing] → [UXprio][needs comment sipaq, review, landing]
Attachment #428487 - Flags: review?(bwinton)
Whiteboard: [UXprio][needs comment sipaq, review, landing] → [UXprio][needs comment sipaq, review bwinton, landing]
Comment on attachment 428487 [details] [diff] [review]
changes the folder pane name of smart folders to unified folders

I think Bryan missed some changes, and will post a new patch soon, which I will review post haste.
Attachment #428487 - Flags: review?(bwinton)
After discussions with Standard8 and clarkbw, the plan is that we'd like for Simon to make the call from comment 3, and if he thinks this is appropriate to land for b1, we would allow this patch to go in tomorrow, despite it being slightly past code freeze.  Otherwise, we'll slip it to b2, and land it as soon as the tree re-opens.
Whiteboard: [UXprio][needs comment sipaq, review bwinton, landing] → [UXprio][needs comment sipaq, updated patch clarkbw, review bwinton, landing]
Please don't break the string freeze for something as trivial as this. I think having this land early in the beta2 cycle is more than fine.
(In reply to comment #3)
> Simon - 
> 
> Because of how the code is structured it seems like we have to change all the
> folder pane header strings as there is no easy way to just change the smart
> folders header string.
> 
> http://mxr.mozilla.org/comm-central/search?string=folderPaneHeader_

Please excuse my ignorance, but why don't we just replace?

- folderPaneHeader_smart=Smart Folders
+ folderPaneHeader_unified=Unified Folders
(In reply to comment #7)
> Otherwise, we'll slip it to b2, and land it as soon
> as the tree re-opens.

Given where we are, moving to b2.
blocking-thunderbird3.1: beta1+ → beta2+
(In reply to comment #9)
> (In reply to comment #3)
> > Simon - 
> > 
> > Because of how the code is structured it seems like we have to change all the
> > folder pane header strings as there is no easy way to just change the smart
> > folders header string.
> > 
> > http://mxr.mozilla.org/comm-central/search?string=folderPaneHeader_
> 
> Please excuse my ignorance, but why don't we just replace?
> 
> - folderPaneHeader_smart=Smart Folders
> + folderPaneHeader_unified=Unified Folders

Yeah, it would be nice if it was that easy but the mode names are referenced in the code.  header = getL10n("folderPaneHeader_" + mode) so I'd have to replace every reference to "smart" in the code with unified.  At first pass it wasn't easy to do.
just checkpointing myself here because I haven't been able to build TB for the Mac I haven't been able to test this patch
Attachment #428487 - Attachment is obsolete: true
Attached patch tested version (obsolete) — Splinter Review
Ok, here's a much nicer and working version of this patch.  I'll detail the major changes below.

The code in mail/base/content/folderPane.js looks for the following entities where ${mode} is replaced with a folder pane mode.

folderPaneHeader_${mode}

This entity has been changed to:
folderPaneModeHeader_${mode}

In mail/locales/en-US/chrome/messenger/featureConfigurator.dtd

Any entity referencing smartFolders was renamed to unifiedFolders.  e.g. 
-<!ENTITY featureConfigurator.smartFolders.heading "Smart folders mode">
+<!ENTITY featureConfigurator.unifiedFolders.heading "Unified Folders mode

However if the entity referenced "smart" mode I didn't rename that to unified as it reflect the code better.  e.g.
-<!ENTITY featureConfigurator.smartFolders.smart.status "Using the &quot;smart folders&quot; mode.">
+<!ENTITY featureConfigurator.unifiedFolders.smart.status "Using the &quot;unified folders&quot; mode.">

I've been testing this out for a while and it works well for me.  I played with the Migration Assistant quite a bit and that works just as it used to.

Let me know if you find any issues.
Attachment #429736 - Attachment is obsolete: true
Attachment #429824 - Flags: review?(bwinton)
Whiteboard: [UXprio][needs comment sipaq, updated patch clarkbw, review bwinton, landing] → [UXprio][has patch, needs review bwinton, landing]
missed one last reference
Attachment #429824 - Attachment is obsolete: true
Attachment #429832 - Flags: review?(bwinton)
Attachment #429824 - Flags: review?(bwinton)
So, I think we might want to change
http://support.mozillamessaging.com/en-US/kb/Thunderbird+3+for+users
http://support.mozillamessaging.com/en-US/kb/New+in+Thunderbird+3
and
http://support.mozillamessaging.com/en-US/kb/Migration+Assistant

And shouldn't we also change mail/locales/en-US/chrome/messenger/messenger.dtd:
<!ENTITY smartFolders.label "Smart">
<!ENTITY smartFolders.accesskey "S">
so that the menu says the same thing as the folder list, and people can search for "Unified" and get a hit?
Attachment #429832 - Flags: review?(bwinton) → review-
I'm going to r- this, based on the questions above.

But, if you wanted to spin the sumomo stuff into a new bug, and make the smartFolders fix, I think I would accept that.
Whiteboard: [UXprio][has patch, needs review bwinton, landing] → [UXprio][needs updated patch]
(In reply to comment #16)
> I'm going to r- this, based on the questions above.
> 
> But, if you wanted to spin the sumomo stuff into a new bug, and make the
> smartFolders fix, I think I would accept that.

Yeah, I'd put the sumomo stuff into a new bug.  I should have an updated patch for this soon.
Ugh, we already have Unread using the 'U' so it looks like 'n'

<!ENTITY unifiedFolders.label "Unified">
<!ENTITY unifiedFolders.accesskey "n">
Ok, here's the final version with fixes to the menu.  Thanks!
Attachment #429832 - Attachment is obsolete: true
Attachment #431653 - Flags: review?(bwinton)
Comment on attachment 431653 [details] [diff] [review]
final version with menu fixes

Looks good to me.  I'ld like it if you added tests, and pasted in here the bug number(s) of the bugs to change sumomo, but I won't block the checkin on those.

Later,
Blake.
Attachment #431653 - Flags: review?(bwinton) → review+
Blocks: 551788
(In reply to comment #20)
> (From update of attachment 431653 [details] [diff] [review])
> Looks good to me.  I'ld like it if you added tests, and pasted in here the bug
> number(s) of the bugs to change sumomo, but I won't block the checkin on those.

Added, depends on this bug.

Can you give me a hint of what I should be testing?  I'm not really sure what to write.  Testing that the string reads as the dtd file suggests seems odd to me.  I could test the new accesskey for Unified folders, but that's my only idea.  Thanks!
doh
Keywords: checkin-needed
Whiteboard: [UXprio][needs updated patch] → [UXprio]
(In reply to comment #21)
> (In reply to comment #20)
> > (From update of attachment 431653 [details] [diff] [review] [details])
> > Looks good to me.  I'ld like it if you added tests, and pasted in here the bug
> > number(s) of the bugs to change sumomo, but I won't block the checkin on those.
> 
> Added, depends on this bug.
> 
> Can you give me a hint of what I should be testing?  I'm not really sure what
> to write.  Testing that the string reads as the dtd file suggests seems odd to
> me.  I could test the new accesskey for Unified folders, but that's my only
> idea.  Thanks!

My main thought with the tests was switching to and from Unified Folders, just to make sure we didn't forget to change an element name somewhere that would cause it to blow up.  But I agree that it's tricky to test.

Thanks,
Blake.
Checked in: http://hg.mozilla.org/comm-central/rev/8f5d55d3a285
Status: NEW → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.1b2
An entity in messenger.properties isn't renamed to "...unified":

folderPaneModeHeader_smart=Unified Folders
(In reply to comment #25)
> An entity in messenger.properties isn't renamed to "...unified":
> 
> folderPaneModeHeader_smart=Unified Folders

That is correct. The internal name of the folder mode is still "smart" - we have to do that for extensions. That is why the prefix changed but not the postfix.
Maybe worth a relnot for people using 3.0.x ?
Keywords: relnote
Keywords: relnote
A very long time ago when 'Smart Folders' were first introduced, I didn't like them due to a number of frustrating limitations (hard-coded, couldn't customize them), and searched for and found the CSS to hide them permanently.

Recently I was forced to re-examine this when helping someone else who was using them in Thunderbird, and after playing around, realized that, with some tweaking, they could be really useful - especially after being forced to help someone else with an Outlook issue...

So, I have opened four bugs for enhancing this folder system: bug 1163562 (first one) and bug 1163555 (last one) are very related, and the first would be automatically 'fixed' if the last one was implemented. Personally I believe that implementing these enhancements would cause this folder system to go from 'interesting concept/idea but needs work before it will be useful to me', to 'awesome power tool that makes Thunderbird really stand out!'...

bug 1163562 - Make Favorites show at top of Folder Pane

bug 1159713 - Allow customization of choices in View > Folders (Unified Folders'

bug 1159716 - Add my own custom 'Unified Folder' choices

bug 1163555 - Allow 'Split' view of multiple 'Unified Folders'
(this one could actually replace and be much more powerful than bug 1163562)

Now... during this time I also discovered this bug that caused 'Smart folders' to be renamed to 'Unified Folders' - but in his opening comment, Bryan didn't elaborate at all on "A number of points of feedback have shown that the term "unified" is clearer than the term "smart" for the smart folders system", and I for one do not understand the rationale for this at all...

Naming them 'Unified Folders', then having one of the views named 'Unified' is confusing. Unified is absolutely the perfect name for the Unified view, but it doesn't make sense as the name for the overall system.

So, I also just opened bug 1163567 to change this name back to 'Smart Folders'. Absent convincing argument to the contrary, I personally think 'Smart Folders' was the ideal term for this folder system, and will be even more appropriate if any of my above referenced bugs/enhancements for improving on it are ever implemented.
You need to log in before you can comment on or make changes to this bug.