477832 - Add a hidden pref for not closing the window with the last tab (mail.tabs.closeWindowWithLastTab)

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, enhancement)

Description

[Przemyslaw Bialik]

Port Bug 455852 to TB.

Comment 1

[Przemyslaw Bialik]

Attachment: Fix v1

Comment 2

[Magnus Melin [:mkmelin]]

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.

Comment 3

[Przemyslaw Bialik]

Attachment: Fix v2

Like this ?

Comment 4

[Magnus Melin [:mkmelin]]

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.

Comment 5

[Przemyslaw Bialik]

Attachment: Patch for the checkin [Checkin: Comment 15]

nit addressed

Comment 6

[Serge Gautherie (:sgautherie)]

Comment on attachment 362461 [details] [diff] [review]
Patch for the checkin
[Checkin: Comment 15]


http://hg.mozilla.org/comm-central/rev/46810acce1c4

Comment 7

[Serge Gautherie (:sgautherie)]

(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

[Bryan Clark (DevTools PM) [:clarkbw]]

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.

Comment 9

[Phil Ringnalda (:philor)]

Backed out in http://hg.mozilla.org/comm-central/rev/6de5d1ad99a8

Comment 10

[Przemyslaw Bialik]

> 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

[Phil Ringnalda (:philor)]

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.

Comment 12

[Przemyslaw Bialik]

(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

[Mark Banner (:standard8)]

(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

[Mark Banner (:standard8)]

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.

Comment 15

[Serge Gautherie (:sgautherie)]

Comment on attachment 362461 [details] [diff] [review]
Patch for the checkin
[Checkin: Comment 15]


http://hg.mozilla.org/comm-central/rev/b4d07b17fb16