Closed Bug 529299 Opened 10 years ago Closed 4 years ago

Presence of accounts using Global Inbox in Folder Location list is confusing

Categories

(Thunderbird :: Toolbars and Tabs, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 50.0

People

(Reporter: nir.sen, Assigned: aceman)

References

Details

Attachments

(1 file, 1 obsolete file)

3.48 KB, patch
Paenglab
: ui-review+
squib
: feedback+
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.4) Gecko/20091027 Fedora/3.5.4-1.fc11 Firefox/3.5.4 XPCOMViewer/0.9a
Build Identifier: 

'Folder Location' gadget includes all accounts irrespective of its type and folder structure in folder pane. As a result even if a folder (Inbox/Trash) of a Global Inbox account is chosen in Folder location list Tb will fail to find any corresponding folder in folder pane and selects main account name in folder pane.


Reproducible: Always

Steps to Reproduce:
1. Configure a POP account to use Global Inbox.
2. Customize Toolbar to include Folder Location gadget.
3. Select any folder or account name of that account in Folder location list.
Actual Results:  
Tb focus on the name of default mail account in folder pane.

Expected Results:  
Either Tb will not display any of account that uses Global Inbox, i.e, Folder location list will mirror same folder tree structure as in folder pane.

Or By selecting any folder of Global Inbox account will select corresponding folder in Local Folders.

I'm using 
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.4pre) Gecko/20091014 Fedora/3.0-2.8.b4.fc11 Thunderbird/3.0b4
Whiteboard: Folder Location selecctor
Version: unspecified → Trunk
I suspect the behaviour of optional toolbar gadgets may not be fine-tuned for all cases. I understand this bug uses Global Inbox, where several pop accounts share the same folder structure within "Local Folders" Account (Tools > Account Settings > Your Account > Server Settings > Advanced > When downloading POP..., use: "Global Inbox (Local Folders Account)"). Can somebody with that setup confirm this bug?
OS: Linux → All
Hardware: x86 → All
Whiteboard: Folder Location selecctor → Folder Location selector
Duplicate of this bug: 527361
This is real. I think this should be easy to fix.
Assignee: nobody → acelists
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: Folder Location selector
Actually this widget shows all folders existing, regardless of which folder view you have. E.g. if change to Favorite View, you only get some folders. But this Location folder widget still shows all. If you select one of the non-visible folders, nothing happens.
Duplicating the logic of displaying folders in the folder pane isn't probably the best idea.
What about at least telling the user "the wanted folder simply can't be shown because it isn't in view"?
Flags: needinfo?(bwinton)
Also the folder pane has the option to switch to "all" view if it couldn't find the wanted folder. We could use that if the user confirms it.

Even in the "All" view the deferred folders/accounts will not be seen. I can look at that. I just say that even if I fix that, the Folder Location is generally broken as it will not find folders if you are in any other than All view. I ask how to fix that part.
Attached patch patch (obsolete) — Splinter Review
Something like this.
Attachment #8606655 - Flags: ui-review?(bwinton)
Interrupting the user to ask them a question they probably don't really care about doesn't seem to me like the right way to go here…

(And you don't need the "aForceSelect = false" change, since if it's not passed in, it'll be undefined, which is like false.  Similarly, down below, you don't need to pass in "false" for the same reason.)
Flags: needinfo?(bwinton)
(In reply to Blake Winton (:bwinton) from comment #7)
> Interrupting the user to ask them a question they probably don't really care
> about doesn't seem to me like the right way to go here…
So do we force the switch without asking? 

> (And you don't need the "aForceSelect = false" change, since if it's not
> passed in, it'll be undefined, which is like false.  Similarly, down below,
> you don't need to pass in "false" for the same reason.)
I do not like undefined things if they can be avoided :)
Comment on attachment 8606655 [details] [diff] [review]
patch

So, I still get the popup when the folder view (on the side) isn't shown, which seems confusing.  So given that, I think forcing the switch is a better choice here.
Attachment #8606655 - Flags: ui-review?(bwinton) → ui-review-
What do you guys think of this?
Flags: needinfo?(squibblyflabbetydoo)
Flags: needinfo?(richard.marti)
Flags: needinfo?(mkmelin+mozilla)
I'm not sure what this bug is asking for, or what this patch is trying to do, but I don't think we should be adding this code to folderPane.js. I don't think that's the right place to put confirmation dialogs.
Flags: needinfo?(squibblyflabbetydoo)
There are already some confirmation dialogs in folderPane.js. But It can probably be moved into the Folder location widget itself.
I think the switch should be forced. The user wants to go to this folder as he selects it in the widget, so this folder should be shown.
Flags: needinfo?(richard.marti)
OK, that is easily doable. It just forces the All folder mode too. That is why the question was there. But we can try it without.
But only force the All folder view when the folder isn't listed in the actual view? I wouldn't like when it switches from Unified view to All folder view also when the folder is present in Unified view.
Yes, that is the idea here, see http://mxr.mozilla.org/comm-central/source/mail/base/content/folderPane.js#511 when aForceSelect=true.
Attached patch patch v2Splinter Review
Attachment #8606655 - Attachment is obsolete: true
Attachment #8744979 - Flags: ui-review?(richard.marti)
Status: NEW → ASSIGNED
Flags: needinfo?(mkmelin+mozilla)
Attachment #8744979 - Flags: ui-review?(richard.marti) → ui-review+
Attachment #8744979 - Flags: review?(squibblyflabbetydoo)
Comment on attachment 8744979 [details] [diff] [review]
patch v2

I'm probably not a great reviewer for this, since I don't really know much about this part of the codebase. However, I don't see anything *wrong* with the patch. If that's good enough for you, feel free to land it. But you might want to get another set of eyes on this. :)
Attachment #8744979 - Flags: review?(squibblyflabbetydoo) → feedback+
Thanks, I think it is enough :)
https://hg.mozilla.org/comm-central/rev/e5a38b702221e989a2e027cf03fd720c3acf6713
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
You need to log in before you can comment on or make changes to this bug.