Closed Bug 516353 Opened 13 years ago Closed 13 years ago

"Get New Messages for > All accounts" is missing

Categories

(Thunderbird :: Mail Window Front End, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0rc1

People

(Reporter: thomas8, Assigned: bwinton)

References

Details

(Keywords: regression, Whiteboard: [no l10n impact])

Attachments

(2 files, 4 obsolete files)

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090912 Shredder/3.1a1pre

TB 2 has a command that checks for new messages on ALL accounts, both in the "Get Mail" button dropdown and in the File > Get new messages for - Menu.
Unfortunately, that command has disappeared in TB3...
Don't tell me it's a feature...?
This is a regression from bug 465269, it appears the folder location menu is now clearing out all items at the wrong time (or not accounting for keeping some items).

Seeing as the folder location menu on the toolbar and the various Get Mail menus use the same widget, we need to account for this.
Blocks: 465269
Flags: blocking-thunderbird3?
Keywords: regression
Summary: "Get New Messages for > All accounts" is missing in TB3 → "Get New Messages for > All accounts" is missing
Assignee: nobody → bwinton
Status: NEW → ASSIGNED
But to do that, we'll need to change _initialized to an attribute, so that it'll get copied when we customize the toolbar.

Note: There's still a bug with sub-folders showing the root elements after you customize the toolbar, because the _parentFolder field also gets magically transmuted into null when you customize the toolbar, but we could probably push the fix for that off to rc1.

Thanks,
Blake.
Attachment #400556 - Flags: superreview?(bienvenu)
Attachment #400556 - Flags: review?(bugmail)
Attachment #400556 - Flags: approval-thunderbird3?
Comment on attachment 400556 [details] [diff] [review]
A patch to not delete our children quite so often.

this fixes the issue for me.
Attachment #400556 - Flags: superreview?(bienvenu) → superreview+
though Neil might have an opinion on the best way to do this.
So this is the previous patch with the addition of converting _parentFolder into an attribute, so that it gets saved, and we avoid the bug I mentioned in the first patch.

I'm not sure whether we want to go with this, or with the previous patch, so I'm not marking this as obsoleting the previous patch.

Thanks,
Blake.
Attachment #400569 - Flags: superreview?(bienvenu)
Attachment #400569 - Flags: review?(bugmail)
Attachment #400569 - Flags: approval-thunderbird3?
As a side note, both patches remove all the account entries from the Get Messages button when you customize the toolbar.  I'm working on it.
Comment on attachment 400569 [details] [diff] [review]
[checked-in] A patch that fixes the bug in the previous patch.

The get mail button thing breaks after I bring up the customize dialog.  The customize dialog is a trouble-maker.
Attachment #400569 - Flags: review?(bugmail) → review-
Comment on attachment 400569 [details] [diff] [review]
[checked-in] A patch that fixes the bug in the previous patch.

a=Standard8 for whichever patch we go with for b4.
Attachment #400569 - Flags: approval-thunderbird3? → approval-thunderbird3+
Attachment #400569 - Flags: superreview?(bienvenu) → superreview+
Comment on attachment 400569 [details] [diff] [review]
[checked-in] A patch that fixes the bug in the previous patch.

some spaces after '?' and ':' would be nice. Most people prefer the '?' on the prev line (not me though :-) )
http://hg.mozilla.org/comm-central/rev/5bddaf7248c8 pushed - not marking fixed because there's more to do, but this is what we want for b4.
Comment on attachment 400569 [details] [diff] [review]
[checked-in] A patch that fixes the bug in the previous patch.

Andrew gave r+ for this over irc for beta 4 as it is restoring the functionality for users the best we can at the moment.
Attachment #400569 - Flags: review- → review+
Flags: blocking-thunderbird3? → blocking-thunderbird3+
Target Milestone: --- → Thunderbird 3.0rc1
Attachment #400556 - Attachment is obsolete: true
Attachment #400556 - Flags: review?(bugmail)
Attachment #400556 - Flags: approval-thunderbird3?
Whiteboard: [no l10n impact]
(In reply to comment #7)
> The get mail button thing breaks after I bring up the customize dialog.  The
> customize dialog is a trouble-maker.

Man, is it ever.  I don't know why closing the pane silently deletes some of my children, but at least now we seem to handle that case a little better.

Thanks,
Blake.
Attachment #401067 - Flags: superreview?(bienvenu)
Attachment #401067 - Flags: review?(bugmail)
Comment on attachment 401067 [details] [diff] [review]
Handle the post-customization case, too.

Okay, so I've looked at this more.  I think the folder widget is making its own troubles and the toolbar customization code is actually fairly blameless.  The folder is doing two things that I think are leading to trouble:

1) Using cloneNode on itself.  We should stop doing that.

2) Forcing itself to be created when its parent is a menulist.  We should not do this when it is in a wrapper.  The check '/wrapper-.*/.test(this.parentNode.parentNode.parentNode.id)' should be sufficient.

With this done, we can avoid trying to persist state in attributes.  Once we stop cloning ourselves, and once we stop trying to be a live XBL binding in the toolbar palette, there should no longer be any 'live' instances that should not be live.

Please make those changes and lose the kludgey attribute stuff.
Attachment #401067 - Flags: superreview?(bienvenu)
Attachment #401067 - Flags: review?(bugmail)
Attachment #401067 - Flags: review-
Attachment #400569 - Attachment description: A patch that fixes the bug in the previous patch. → [checked-in] A patch that fixes the bug in the previous patch.
Depends on: 517134
(In reply to comment #13)
> (From update of attachment 401067 [details] [diff] [review])
> Okay, so I've looked at this more.  I think the folder widget is making its
> own troubles and the toolbar customization code is actually fairly blameless.

Sure.  Not totally blameless, but fairly blameless.  :)

> The folder is doing two things that I think are leading to trouble:
> 1) Using cloneNode on itself.  We should stop doing that.

Stopped.

> 2) Forcing itself to be created when its parent is a menulist.  We should not
> do this when it is in a wrapper.  The check
> '/wrapper-.*/.test(this.parentNode.parentNode.parentNode.id)' should be
> sufficient.

Checked.

With these two changes, we were still getting created multiple times, with new field values and with old children, so while this was some good cleanup, it didn't fix the original problem.

But, now that I can tell when it's being put into a wrapper, I can clear out its children, and then when it gets re-created, we only have one copy of them, which is what we want.

> With this done, we can avoid trying to persist state in attributes.  Once we
> stop cloning ourselves, and once we stop trying to be a live XBL binding in
> the toolbar palette, there should no longer be any 'live' instances that
> should not be live.
> 
> Please make those changes and lose the kludgey attribute stuff.

Done, and done.

As a side note, I've also tested this with the newsgroup message folder stuff, and the newsgroups appear.  And when I add and remove servers, it all seems to work correctly.

Finally, since we're no longer using GetMsgFolderFromUri, I think this might also fix bug 517134, without applying Kent's patch.

Thanks,
Blake.
Attachment #401067 - Attachment is obsolete: true
Attachment #401445 - Flags: superreview?(bienvenu)
Attachment #401445 - Flags: review?(bugmail)
Whiteboard: [no l10n impact] → [no l10n impact][patch up]
Attachment #401445 - Flags: superreview?(bienvenu)
Attachment #401445 - Flags: review?(bugmail)
Comment on attachment 401445 [details] [diff] [review]
Let's try a different tack on this.

Found a bug.  Will fix.
So the regex asuth gave works most of the time, but not for the File menu-button, because it's got its menupopup wrapped in another node, so we didn't see that it was being wrapped.  So now we loop through all our parents, checking for wrappers.

Thanks,
Blake.
Attachment #401445 - Attachment is obsolete: true
Attachment #401850 - Flags: superreview?(bienvenu)
Attachment #401850 - Flags: review?(bugmail)
Depends on: 517617
Comment on attachment 401850 [details] [diff] [review]
The previous patch, with the bug fixed.

It's in the file menu too?  This menu sure gets around.

Thank you for the stick-to-itiveness on this bug and driving the ghosts out.
Attachment #401850 - Flags: review?(bugmail) → review+
Attachment #401850 - Flags: superreview?(bienvenu) → superreview+
Comment on attachment 401850 [details] [diff] [review]
The previous patch, with the bug fixed.

period at the end of this sentence:

+        // If we are a child of a menulist, and we aren't in a wrapper, we
+        // need to build our content right away, otherwise the menulist
+        // won't have proper sizing

"because" here:

+        // But if we're in a wrapper, remove our children, cause we're
+        // getting re-created when the toolbar customization closes.
(In reply to comment #17)
> It's in the file menu too?  This menu sure gets around.

Yeah, and the message and newsgroup filters too, apparently.

> Thank you for the stick-to-itiveness on this bug and driving the ghosts out.

My pleasure.  :)

(In reply to comment #18)
> period at the end of this sentence:

Fixed.

> "because" here:

And fixed.

Thanks,
Blake.
Attachment #401850 - Attachment is obsolete: true
Keywords: checkin-needed
Whiteboard: [no l10n impact][patch up] → [no l10n impact]
Checked in: http://hg.mozilla.org/comm-central/rev/2aea95c35322
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Duplicate of this bug: 518586
patch is landed?

Here the bug is still open on

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4pre) Gecko/20090926 Lightning/1.0pre Shredder/3.0pre ID:20090926031214

STR:
1. rx click on toolbar and select "custumize..."
2. on "customize toolbar" window click done button.

goto "Get Mail" toolbar button: "get all new messages" is missing.

When restart TB, all is fine.
(In reply to comment #22)
> patch is landed?
> 
> Here the bug is still open on

That's a slightly different scenario (this one was that get all new messages was completely missing all the time), please file a new bug for it.
You need to log in before you can comment on or make changes to this bug.