View -> Threads -> Threads with unread mode does not persist

RESOLVED FIXED in Thunderbird 3.0b3

Status

Thunderbird
Mail Window Front End
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Usul, Assigned: asuth)

Tracking

(Blocks: 1 bug, {regression})

Trunk
Thunderbird 3.0b3
regression
Bug Flags:
blocking-thunderbird3 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

9 years ago
With the try server build When I do that I get the following dump :

An error occurred executing the cmd_viewThreadsWithUnread command
ReferenceError: gCurrentViewValue is not defined
(Assignee)

Comment 1

9 years ago
Yeah, two lines of code in SwitchView were not updated and were trying to use old globals.  I've manually tested the fix.
(Reporter)

Comment 2

9 years ago
(In reply to comment #1)
> Yeah, two lines of code in SwitchView were not updated and were trying to use
> old globals.  I've manually tested the fix.

I'm by default to Threads with unread. If I switch to All - I see all my messages - but if want to switch back to my default it stays in All. I can only change the sorting once per session I need to restart TB in order to be able to change the ordering.

Tools -> Error console is empty (Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090613 Shredder/3.0b3pre)

Comment 3

9 years ago
Same here. Here is what I get in error console :

Discovering folders for account failed with exception: [Exception... "Component returned failure code: 0x80550013 [nsIMsgFolder.subFolders]"  nsresult: "0x80550013 (<unknown>)"  location: "JS frame :: chrome://messenger/content/folderPane.js :: ftv__mg_all :: line 1027"  data: no]

Linux box.
(Reporter)

Updated

9 years ago
OS: Mac OS X → All

Comment 4

9 years ago
keyword "REGRESSION"?
Flags: blocking-thunderbird3?
(Reporter)

Comment 5

9 years ago
(In reply to comment #4)
> keyword "REGRESSION"?

Feel free to add it yourself next time :-) I initally did not as this bug is linked to a regression tracker one.
Keywords: regression

Updated

8 years ago
Flags: blocking-thunderbird3? → blocking-thunderbird3+
Hardware: x86 → All
Target Milestone: --- → Thunderbird 3.0b4

Updated

8 years ago
Target Milestone: Thunderbird 3.0b4 → Thunderbird 3.0b3

Comment 6

8 years ago
As of today, view threads with unread works when I switch to it, but it's not sticky, so if I leave the folder and come back, I'm back to view | all. Ludovic, is that what you're seeing? I can look into it, unless asuth knows the fix off the top of his head.
I see the same behavior described in comment 6; I think we're going to need this for b3.
Andrew, can you have a look at this?
Assignee: nobody → bugmail
Summary: Can't switch to View -> Threads -> Threads with unread → View -> Threads -> Threads with unread mode does not persist
(Assignee)

Comment 9

8 years ago
We don't restore/consult viewType, we need to.  nsMsgDBView::PersistFolderInfo has the logic that persists it and nsMsgThreadedDBView::Open calls that method for us.  (And the special views subclass nsMsgThreadeDBView.)

This is a problem in DBViewWrapper and can be tested as an xpcshell test.
Status: NEW → ASSIGNED
(Assignee)

Comment 10

8 years ago
Created attachment 386284 [details] [diff] [review]
Check dbFolderInfo.viewType and handle special views, with unit test v1.

The new unit test fails without the patch and passes with it.

Note that the patch only checks viewType for the special view enumeration values.  The rationale is that we have to check viewType for these because it's the only way to know about the special views, but that we don't want/need to trust it for any of the other cases.  For the other view modes, the viewFlags indicate that the view is active, although mail views can impact it.

I've added a comment to _setSpecialView that's a bit of a cop-out comment in the recognition that special views have serious limitations (no searches are possible) that we depend on the UI to handle for us.  The UI does some of this (it clears the mail view when you select one of the special views), but it allows things to get into confusing states and that needs to be spun off into a different bug.  (It doesn't clear the quick-search, it doesn't disable the mail-views menus, it doesn't disable quick-search.)
Attachment #386284 - Flags: review?(bienvenu)

Comment 11

8 years ago
Comment on attachment 386284 [details] [diff] [review]
Check dbFolderInfo.viewType and handle special views, with unit test v1.

with this patch, I can't get back to view | threads | all, which is making me very sad :-(

Comment 12

8 years ago
restart does fix it, though. Which is nice, but maybe a bug as well.

Comment 13

8 years ago
Comment on attachment 386284 [details] [diff] [review]
Check dbFolderInfo.viewType and handle special views, with unit test v1.

minusing based on problems running w/ patch.
Attachment #386284 - Flags: review?(bienvenu) → review-
(Assignee)

Comment 14

8 years ago
Created attachment 386442 [details] [diff] [review]
restore special views v2

Right, writes to showUnreadOnly were not clearing the special views.  I've amended that logic to actually do what makes sense in the case of the UI's usage and added a unit test to test_viewWrapper_logic.js that tests this.

While human-testing (following your lead) I noticed that the "View... Threads... Unread" option was no longer doing sane things.  This was another casualty of not just rebuilding the view every time.  I added kUnreadOnly to the list of flag changes that single-folder views don't know how to handle. 

It looks like XFVF has no clue about "kUnreadOnly" and quick searches ignore it (presumably because it would need to get in their search terms).  I have accordingly moved the "cmd_viewUnreadMsgs" option to also be disabled for virtual folders (just like the special views are).  Obviously we could just change that dude to activate the unread mail view, but that seems too wacky.

Assuming this gets r+ and you are fine with making any changes you request, please make them and land it.  Otherwise I'll get to it on Monday.
Attachment #386284 - Attachment is obsolete: true
Attachment #386442 - Flags: review?(bienvenu)

Comment 15

8 years ago
Comment on attachment 386442 [details] [diff] [review]
restore special views v2

this seems to be a big improvement - I'll land this today.
Attachment #386442 - Flags: review?(bienvenu) → review+

Comment 16

8 years ago
fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.