Closed
Bug 477832
Opened 15 years ago
Closed 15 years ago
Add a hidden pref for not closing the window with the last tab (mail.tabs.closeWindowWithLastTab)
Categories
(Thunderbird :: Toolbars and Tabs, enhancement)
Thunderbird
Toolbars and Tabs
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.0b3
People
(Reporter: maxxmozilla, Assigned: maxxmozilla)
References
Details
Attachments
(1 file, 2 obsolete files)
1.72 KB,
patch
|
maxxmozilla
:
review+
|
Details | Diff | Splinter Review |
Port Bug 455852 to TB.
Assignee | ||
Comment 1•15 years ago
|
||
Attachment #361543 -
Flags: review?(mkmelin+mozilla)
Comment 2•15 years ago
|
||
Comment on attachment 361543 [details] [diff] [review] Fix v1 >+ if (tabmail.tabInfo.length == 1 && pref.getBoolPref("mail.tabs.closeWindowWithLastTab")) > window.close(); > else > tabmail.removeCurrentTab(); > break; So, currently if the pref is set to false you'd still call tabmail.removeCurrentTab(). Doesn't do anything bad atm, but I'd prefer you move the pref check inside that if-clause.
Assignee | ||
Comment 3•15 years ago
|
||
Like this ?
Attachment #361543 -
Attachment is obsolete: true
Attachment #362409 -
Flags: review?(mkmelin+mozilla)
Attachment #361543 -
Flags: review?(mkmelin+mozilla)
Comment 4•15 years ago
|
||
Comment on attachment 362409 [details] [diff] [review] Fix v2 Yup, thx! Nit: the else should have { } too, as our style likes for all the same-level clauses to have braces if one of them has.
Attachment #362409 -
Flags: review?(mkmelin+mozilla) → review+
Updated•15 years ago
|
Summary: Add a hidden pref for not closing the window with the last tab → Add a hidden pref for not closing the window with the last tab (mail.tabs.closeWindowWithLastTab)
Assignee | ||
Comment 5•15 years ago
|
||
nit addressed
Attachment #362409 -
Attachment is obsolete: true
Attachment #362461 -
Flags: review+
Assignee | ||
Updated•15 years ago
|
Keywords: checkin-needed
Comment 6•15 years ago
|
||
Comment on attachment 362461 [details] [diff] [review] Patch for the checkin [Checkin: Comment 15] http://hg.mozilla.org/comm-central/rev/46810acce1c4
Attachment #362461 -
Attachment description: Patch for the checkin → Patch for the checkin
[Checkin: Comment 6]
Updated•15 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Depends on: 455852
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b2
Comment 7•15 years ago
|
||
(In reply to comment #6) > http://hg.mozilla.org/comm-central/rev/46810acce1c4 Oh ! I think this needed approval for 30b2 :-< (I saw no TB approval flag to request though :-/) Please, post-approve or let me know if you prefer that I back it out.
Comment 8•15 years ago
|
||
we're in the slushy code/string freeze right now, only items blocking the TB3b2 release can get committed. This will probably need to be backed out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 9•15 years ago
|
||
Backed out in http://hg.mozilla.org/comm-central/rev/6de5d1ad99a8
Status: REOPENED → ASSIGNED
Target Milestone: Thunderbird 3.0b2 → Thunderbird 3.0b3
Assignee | ||
Updated•15 years ago
|
Attachment #362461 -
Attachment description: Patch for the checkin
[Checkin: Comment 6] → Patch for the checkin
Assignee | ||
Comment 10•15 years ago
|
||
> Backed out in http://hg.mozilla.org/comm-central/rev/6de5d1ad99a8
This is not my fault, right ?
Even if I would knew that approval is needed how should it be requested ?
Comment 11•15 years ago
|
||
Nope, not your fault: you could have earned extra credit by constantly checking http://tinderbox.mozilla.org/showbuilds.cgi?tree=Thunderbird3.0 and removing the checkin-needed keyword with a comment about how you would add it back in when the tree reopened, but you are absolutely not required to do that: it's up to people doing checkin to check whether or not something checkin-needed can go in with the tree rules at the time. Practically speaking, you request approval by requesting blocking: we'd really very much rather not take anything other than our still-too-many blockers during this last-Thursday through tomorrow slushy freeze, to give them as quiet a tree as possible to land in, so while it's possible to negotiate approval on IRC or by directly emailing a driver (probably clarkbw, since he's driving this beta), it's not as easy as the months-long periods when you just automatically request approval with a flag on every single patch affecting Firefox.
Assignee | ||
Comment 12•15 years ago
|
||
(In reply to comment #11) > Practically speaking, you request approval by requesting blocking: I know this bug is not the best example but in this case I would have to receive blocking‑thunderbird3+ and only then I can ask (irc, e-mail) for approval when the tree is closed ? > request approval with a flag on every single patch affecting Firefox. why not have beta approval flags for Thunderbird ?
Comment 13•15 years ago
|
||
(In reply to comment #12) > (In reply to comment #11) > > Practically speaking, you request approval by requesting blocking: > I know this bug is not the best example but in this case I would have to > receive blocking‑thunderbird3+ and only then I can ask (irc, e-mail) for > approval when the tree is closed ? We haven't done this until now, because when we're in the end-game of a release, we want to really limit our extra commits - we intend to be in the restricted/closed state for a really short period of time (i.e. a few days, no more than a week) and that's the case where we do want to land just blockers and not "general" nice-to-have bugs - i.e. to completely reduce our exposure to potential regressions. Once we branch comm-central and get closer to TB 3 final, I expect we will introduce an approval flag at that stage.
Comment 14•15 years ago
|
||
Sorry for bugspam, I should have said we should move this discussion to mozilla.dev.apps.thunderbird if there's anything else to discuss on the approval flag comments.
Assignee | ||
Updated•15 years ago
|
Keywords: checkin-needed
Comment 15•15 years ago
|
||
Comment on attachment 362461 [details] [diff] [review] Patch for the checkin [Checkin: Comment 15] http://hg.mozilla.org/comm-central/rev/b4d07b17fb16
Attachment #362461 -
Attachment description: Patch for the checkin → Patch for the checkin
[Checkin: Comment 15]
Updated•15 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 15 years ago → 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: Thunderbird 3.0b3 → Thunderbird 3.0b2
Updated•15 years ago
|
Target Milestone: Thunderbird 3.0b2 → Thunderbird 3.0b3
You need to log in
before you can comment on or make changes to this bug.
Description
•