Last Comment Bug 386363 - Remember zoom value on a site-specific basis (Port fix for bug 378549 to SeaMonkey)
: Remember zoom value on a site-specific basis (Port fix for bug 378549 to SeaM...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: -- enhancement with 7 votes (vote)
: seamonkey2.1b2
Assigned To: Robert Kaiser (not working on stability any more)
:
Mentors:
: 581043 (view as bug list)
Depends on: 378549 588895
Blocks: 519120
  Show dependency treegraph
 
Reported: 2007-06-29 12:52 PDT by Raj Bhaskar
Modified: 2013-01-01 03:26 PST (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
WIP state, works but spits out some errors in mailnews usage (42.53 KB, patch)
2010-08-19 12:30 PDT, Robert Kaiser (not working on stability any more)
no flags Details | Diff | Review
v1: switch to toolkit ZoomManager, save site-specific zoom (29.26 KB, patch)
2010-09-05 08:22 PDT, Robert Kaiser (not working on stability any more)
neil: review-
Details | Diff | Review
v1.1: use onLocationChange, explain pref better (29.00 KB, patch)
2010-12-01 17:19 PST, Robert Kaiser (not working on stability any more)
no flags Details | Diff | Review
v1.2: fix further review comments (26.75 KB, patch)
2010-12-07 12:31 PST, Robert Kaiser (not working on stability any more)
no flags Details | Diff | Review
v1.3: fixes for (hopefully) last two comments (26.67 KB, patch)
2010-12-09 08:26 PST, Robert Kaiser (not working on stability any more)
neil: review+
Details | Diff | Review

Description Raj Bhaskar 2007-06-29 12:52:36 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4) Gecko/20070509 MultiZilla/1.8.3.2i SeaMonkey/1.1.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4) Gecko/20070509 MultiZilla/1.8.3.2i SeaMonkey/1.1.2

Bug 108391 -- text size (zoom) should be remembered across browser sessions, per-site -- was fixed by bug 378549 for Firefox.  According to bug 108391 comment 50 this is for Firefox only but that the content prefs service is Toolkit so can be ported to SuiteRunner.

Reproducible: Didn't try
Comment 1 Tony Mechelynck [:tonymec] 2008-04-18 18:25:26 PDT
This is still a port of Firefox bug 378549 but I'm copying the latter's Summary to make it clearer what this bug is about.

Sm-trunk prefs are indeed being ported to a Toolkit backend: some parts of it are already done, others not. I'm not sure where this belongs in Sm prefs but if it's in Appearance or one of its subpanes they are already ported.

Adding a few CC but not sure I made the right choices -- feel free to add others or to remove yourself.
Comment 2 Philip Chee 2008-04-18 21:34:09 PDT
Just putting back the link to the firefox bug in the summary.
Comment 3 Philip Chee 2008-04-18 21:34:46 PDT
Do we have Myk's site preference service on trunk?
Comment 4 Jens Hatlak (:InvisibleSmiley) 2009-06-16 22:42:58 PDT
(In reply to comment #3)
> Do we have Myk's site preference service on trunk?

The patch from bug 378547 only touched toolkit/ so I guess the answer is Yes (I didn't check any further, though).
Comment 5 Philip Chee 2010-07-22 09:26:54 PDT
*** Bug 581043 has been marked as a duplicate of this bug. ***
Comment 6 Robert Kaiser (not working on stability any more) 2010-07-22 09:46:05 PDT
http://mxr.mozilla.org/comm-central/source/mozilla/browser/base/content/browser-fullZoom.js
is where this functionality sits in Firefox, and this is included into
browser.js - should not be too hard to port this over.

BTW, the content prefs this sets can be managed with the new Data Manager I'm
developing.
Comment 7 Robert Kaiser (not working on stability any more) 2010-07-31 12:34:15 PDT
I have a version of this working on the browser side, including a switch to toolkit's ZoomManager, but I need to do some more work for mailnews (ZommManager now uses getBrowser() and that seems to not exist there atm).
Comment 8 Misak Khachatryan 2010-08-16 06:43:40 PDT
As part of work on bug 583317, there is likely relevant code in suite/browser/nsBrowserStatusHandler.js, leaving it here as a memo to include as part of patch for this bug. I'm going to remove it from my patch as it fills my console with errors.

in function onUpdateCurrentBrowser:

+  onUpdateCurrentBrowser: function (aStateFlags, aStatus, aMessage, aTotalProgress) {
+    if (FullZoom.updateBackgroundTabs)
+      FullZoom.onLocationChange(gBrowser.currentURI, true);
Comment 9 Robert Kaiser (not working on stability any more) 2010-08-16 06:53:03 PDT
(In reply to comment #8)
> +    if (FullZoom.updateBackgroundTabs)
> +      FullZoom.onLocationChange(gBrowser.currentURI, true);

Yes, leave those out of your patch, I'm adding that over here.
Comment 10 Robert Kaiser (not working on stability any more) 2010-08-19 12:30:28 PDT
Created attachment 467499 [details] [diff] [review]
WIP state, works but spits out some errors in mailnews usage

Here's a WIP patch as a checkpoint. With the patches in the dependencies applied, this works, even in mailnews, but when used there, it currently leaves error messages in the error console about gInPrintPreviewMode being not defined when only used in the if () clauses - which confuses me because I thought this being not defined would just be interpreted like false (actually undefined) in an if.
Comment 11 Robert Kaiser (not working on stability any more) 2010-08-19 12:37:56 PDT
Hah, attaching a patch and looking at it makes one see interesting stuff - the addition of fullZoom.js is a remnant and not needed, it's dead even in the patch. Removed locally.
Comment 12 Robert Kaiser (not working on stability any more) 2010-09-05 08:22:04 PDT
Created attachment 472268 [details] [diff] [review]
v1: switch to toolkit ZoomManager, save site-specific zoom

This patch depends on bug 583317 to apply changed prefs when changing tabs.

Apart from that, nobody told me here, but I finally found out that using window.varname, I can catch undefined vars (nicely, I can check for the var being true or false when actually defined at the same time).

With that, this patch seems to work fine in both browser an mailnews.
Comment 13 neil@parkwaycc.co.uk 2010-09-05 09:06:31 PDT
Comment on attachment 472268 [details] [diff] [review]
v1: switch to toolkit ZoomManager, save site-specific zoom

>   // simulate all change notifications after switching tabs
>   onUpdateCurrentBrowser: function (aStateFlags, aStatus, aMessage, aTotalProgress) {
>+    if (FullZoom.updateBackgroundTabs)
>+      FullZoom.onLocationChange(gBrowser.currentURI, true);
I don't understand this; onUpdateCurrentBrowser only fires for the foreground tab. I also couldn't see where else updateBackgroundTabs gets used.

You can avoid the dependency on the other bug by moving the tab switch update code to the same place as the star button.
Comment 14 Robert Kaiser (not working on stability any more) 2010-09-05 09:24:21 PDT
(In reply to comment #13)
> >   // simulate all change notifications after switching tabs
> >   onUpdateCurrentBrowser: function (aStateFlags, aStatus, aMessage, aTotalProgress) {
> >+    if (FullZoom.updateBackgroundTabs)
> >+      FullZoom.onLocationChange(gBrowser.currentURI, true);
> I don't understand this; onUpdateCurrentBrowser only fires for the foreground
> tab. I also couldn't see where else updateBackgroundTabs gets used.

It gets fired when background tabs that should be updated come into the foreground, right? While they're in the background, we don't need to update them, as they're not visible and we don't need to waste perf on them, but when they come into the foreground, we need to update their zoom level.

> You can avoid the dependency on the other bug by moving the tab switch update
> code to the same place as the star button.

That would make us depart from the code paths that FF uses as well, I guess you rarely do code-porting jobs where the code needs to be maintained into the future. OTOH, we are so far off with a lot of this code already that nobody who is sane can follow code changes on their side anyhow to backport them.
Comment 15 neil@parkwaycc.co.uk 2010-09-05 14:05:49 PDT
(In reply to comment #14)
> (In reply to comment #13)
> > >   // simulate all change notifications after switching tabs
> > >   onUpdateCurrentBrowser: function (aStateFlags, aStatus, aMessage, aTotalProgress) {
> > >+    if (FullZoom.updateBackgroundTabs)
> > >+      FullZoom.onLocationChange(gBrowser.currentURI, true);
> > I don't understand this; onUpdateCurrentBrowser only fires for the foreground
> > tab. I also couldn't see where else updateBackgroundTabs gets used.
> It gets fired when background tabs that should be updated come into the
> foreground, right? While they're in the background, we don't need to update
> them, as they're not visible and we don't need to waste perf on them, but when
> they come into the foreground, we need to update their zoom level.
That sounds great, but I don't see where you avoid updating tabs that are in the background.

> > You can avoid the dependency on the other bug by moving the tab switch update
> > code to the same place as the star button.
> That would make us depart from the code paths that FF uses as well, I guess you
> rarely do code-porting jobs where the code needs to be maintained into the
> future. OTOH, we are so far off with a lot of this code already that nobody who
> is sane can follow code changes on their side anyhow to backport them.
Ah, so Firefox developers having arbitrarily decided whether a particular UI update (e.g. PlacesStarButton) belongs either in onLocationChange or onUpdateCurrentBrowser, we then have to slavishly follow their decision, rather than keeping all our UI updates in one place?
Comment 16 Robert Kaiser (not working on stability any more) 2010-09-05 14:30:15 PDT
(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > >   // simulate all change notifications after switching tabs
> > > >   onUpdateCurrentBrowser: function (aStateFlags, aStatus, aMessage, aTotalProgress) {
> > > >+    if (FullZoom.updateBackgroundTabs)
> > > >+      FullZoom.onLocationChange(gBrowser.currentURI, true);
> > > I don't understand this; onUpdateCurrentBrowser only fires for the foreground
> > > tab. I also couldn't see where else updateBackgroundTabs gets used.
> > It gets fired when background tabs that should be updated come into the
> > foreground, right? While they're in the background, we don't need to update
> > them, as they're not visible and we don't need to waste perf on them, but when
> > they come into the foreground, we need to update their zoom level.
> That sounds great, but I don't see where you avoid updating tabs that are in
> the background.

I don't see a place I added here that actually updates background tabs. Is that enough?

> > > You can avoid the dependency on the other bug by moving the tab switch update
> > > code to the same place as the star button.
> > That would make us depart from the code paths that FF uses as well, I guess you
> > rarely do code-porting jobs where the code needs to be maintained into the
> > future. OTOH, we are so far off with a lot of this code already that nobody who
> > is sane can follow code changes on their side anyhow to backport them.
> Ah, so Firefox developers having arbitrarily decided whether a particular UI
> update (e.g. PlacesStarButton) belongs either in onLocationChange or
> onUpdateCurrentBrowser, we then have to slavishly follow their decision, rather
> than keeping all our UI updates in one place?

Erm, onLocationChange is only fired when the location of a tab changes, or only if the location of the location bar changes? So, say, I have two tabs open with the same URL and zoom one of them, having the default setting of site-specific zoom on. Now I switch to the other one. Does onLocationChange fire so I can update the zoom level of that tab?
Comment 17 neil@parkwaycc.co.uk 2010-09-07 02:59:16 PDT
(In reply to comment #16)
> (In reply to comment #15)
> > That sounds great, but I don't see where you avoid updating tabs that are in
> > the background.
> I don't see a place I added here that actually updates background tabs. Is that
> enough?
Then I don't understand the point of the pref at all.

> Erm, onLocationChange is only fired when the location of a tab changes, or only
> if the location of the location bar changes? So, say, I have two tabs open with
> the same URL and zoom one of them, having the default setting of site-specific
> zoom on. Now I switch to the other one. Does onLocationChange fire so I can
> update the zoom level of that tab?
Yes, it does fire whenever the location bar changes. (Otherwise the PlacesStarButton wouldn't update correctly!)
Comment 18 Robert Kaiser (not working on stability any more) 2010-09-07 03:43:50 PDT
(In reply to comment #17)
> (In reply to comment #16)
> > (In reply to comment #15)
> > > That sounds great, but I don't see where you avoid updating tabs that are in
> > > the background.
> > I don't see a place I added here that actually updates background tabs. Is that
> > enough?
> Then I don't understand the point of the pref at all.

Well, I just ported having the pref - I for myself am fully happy with always updating tabs that come from the background, regardless of a pref.

> > Erm, onLocationChange is only fired when the location of a tab changes, or only
> > if the location of the location bar changes? So, say, I have two tabs open with
> > the same URL and zoom one of them, having the default setting of site-specific
> > zoom on. Now I switch to the other one. Does onLocationChange fire so I can
> > update the zoom level of that tab?
> Yes, it does fire whenever the location bar changes. (Otherwise the
> PlacesStarButton wouldn't update correctly!)

Well, so, how does the location bar update when I switch between two tabs that that have the same page loaded, then?
Comment 19 neil@parkwaycc.co.uk 2010-09-10 04:55:16 PDT
(In reply to comment #18)
> > Then I don't understand the point of the pref at all.
> Well, I just ported having the pref - I for myself am fully happy with always
> updating tabs that come from the background, regardless of a pref.
I guess I'd better read the Firefox code to see whether it makes sense...

> > Yes, it does fire whenever the location bar changes. (Otherwise the
> > PlacesStarButton wouldn't update correctly!)
> Well, so, how does the location bar update when I switch between two tabs that
> that have the same page loaded, then?
OK, so I was being a bit simplistic: it fires when the location bar might change (so for instance it fires when you browse Google Maps).
Comment 20 neil@parkwaycc.co.uk 2010-09-10 04:58:18 PDT
(In reply to comment #19)
> I guess I'd better read the Firefox code to see whether it makes sense...
No, it makes no sense; the pref seems to be utterly useless.
Comment 21 neil@parkwaycc.co.uk 2010-09-10 04:59:26 PDT
Comment on attachment 472268 [details] [diff] [review]
v1: switch to toolkit ZoomManager, save site-specific zoom

Minusing because the updateBackgroundTabs pref doesn't work.

Note: An updateBackgroundTabs pref is not necessary for the initial feature.
Comment 22 Robert Kaiser (not working on stability any more) 2010-12-01 13:36:19 PST
(In reply to comment #21)
> Comment on attachment 472268 [details] [diff] [review]
> v1: switch to toolkit ZoomManager, save site-specific zoom
> 
> Minusing because the updateBackgroundTabs pref doesn't work.
> 
> Note: An updateBackgroundTabs pref is not necessary for the initial feature.

I actually disagree, as that's one of the great things this feature does. But if SeaMonkey sucks, what do I care, just one reason more to switch myself to Firefox, as much as it will hurt my pride for a while.
Comment 23 neil@parkwaycc.co.uk 2010-12-01 15:42:56 PST
(In reply to comment #21)
> (From update of attachment 472268 [details] [diff] [review])
> Minusing because the updateBackgroundTabs pref doesn't work.
So, it turns out that this pref has nothing to do with updating tabs in the background, or updating tabs that are in the background. Instead its job is to update tabs that were in the background. No wonder I was confused...
Comment 24 Robert Kaiser (not working on stability any more) 2010-12-01 17:19:18 PST
Created attachment 494578 [details] [diff] [review]
v1.1: use onLocationChange, explain pref better

Yes, that's exactly it. :)

This patch uses onLocationChange and tries to explain the pref better, I hope that's in a shape that we can take it.
Comment 25 neil@parkwaycc.co.uk 2010-12-05 14:26:12 PST
Comment on attachment 494578 [details] [diff] [review]
v1.1: use onLocationChange, explain pref better

>@@ -377,16 +377,20 @@ onLocationChange : function
>+    // When background tab comes into foreground, might want to update zoom.
>+    if (FullZoom.updateBackgroundTabs)
>+      FullZoom.onLocationChange(gBrowser.currentURI, true);
There are three problems with this. The first is that you're catching changes for frames, not just top-level changes. Fortunately there's a convenient block of code that already listens for top-level changes where you could move this. Secondly this listens for foreground as well as tab-switch changes. The difference is that aRequest is null for a tab-switch change. So you could pass !aRequest as the aIsTabSwitch parameter and forget the all tabs listener. Finally you don't pass the browser to onLocationChange. I'm concerned that this will cause problems if you switch tabs quickly. Can you not pass in browser (conveniently defined earlier in onLocationChange)?

>+  _handleMouseScrolled: function FullZoom__handleMouseScrolled(event) {
[I don't like the internal double underscores]

>+  onLocationChange: function FullZoom_onLocationChange(aURI, aIsTabSwitch, aBrowser) {
>+    if (!aURI || (aIsTabSwitch && !this.siteSpecific))
When site specific preferences are off, what happens in a non-tab-switch case? All I can see is calls to _applyPrefToSetting which test for siteSpecific too.

>+      let isSaneURI = (aBrowser && aBrowser.currentURI) ?
>+        aURI.equals(aBrowser.currentURI) : false;
aURI.equals(null) is false, so no need to null-test currentURI.

>+      if (!aBrowser || isSaneURI)
You could then go on to replace the three lines with
!aBrowser || aURI.equals(aBrowser.currentURI)

>+  Services.prefs.setCharPref("toolkit.zoomManager.zoomValues",
>+                             zoomBundle.getString("values").split(",")
>+                                       .map(function(aVal) {return aVal/100;})
>+                                       .join(","));
Why not use zoomFactors instead of splitting twice?

>+  var factorOther = zoomOther.value || zoomBundle.getString("valueOther");
IIRC zoomOther.value doesn't work on the Mac, use getAttribute instead.

>+      if (item.getAttribute("value") == parseInt(ZoomManager.zoom * 100))
Should we make it so that the value attribute is the zoom as a fraction?
Comment 26 neil@parkwaycc.co.uk 2010-12-05 15:56:24 PST
Comment on attachment 494578 [details] [diff] [review]
v1.1: use onLocationChange, explain pref better

>+  var zoomType =  ZoomManager.useFullZoom ? "fullZoom" : "textZoom";
Nit: double space

>+  var menuLabel = zoomBundle.getString(zoomType).replace(/%zoom%/, parseInt(ZoomManager.zoom * 100));
...
>+      if (item.getAttribute("value") == parseInt(ZoomManager.zoom * 100))
These need to round. Otherwise 90% becomes 89% (89.99999976158142+).
Comment 27 neil@parkwaycc.co.uk 2010-12-06 05:29:10 PST
(In reply to comment #26)
>(From update of attachment 494578 [details] [diff] [review])
>>+  var menuLabel = zoomBundle.getString(zoomType).replace(/%zoom%/, parseInt(ZoomManager.zoom * 100));
>...
>>+      if (item.getAttribute("value") == parseInt(ZoomManager.zoom * 100))
>These need to round. Otherwise 90% becomes 89% (89.99999976158142+).
Or you could consider toFixed() or toPrecision().
Comment 28 Robert Kaiser (not working on stability any more) 2010-12-07 12:23:20 PST
(In reply to comment #25)
> There are three problems with this. The first is that you're catching changes
> for frames, not just top-level changes. Fortunately there's a convenient block
> of code that already listens for top-level changes where you could move this.
> Secondly this listens for foreground as well as tab-switch changes. The
> difference is that aRequest is null for a tab-switch change. So you could pass
> !aRequest as the aIsTabSwitch parameter and forget the all tabs listener.
> Finally you don't pass the browser to onLocationChange. I'm concerned that this
> will cause problems if you switch tabs quickly. Can you not pass in browser
> (conveniently defined earlier in onLocationChange)?

I guess using onUpdateCurrentBrowser would have solved all those. :P
Still, I'll just apply all those changes you are wishing for here and hope it works out.

> >+  onLocationChange: function FullZoom_onLocationChange(aURI, aIsTabSwitch, aBrowser) {
> >+    if (!aURI || (aIsTabSwitch && !this.siteSpecific))
> When site specific preferences are off, what happens in a non-tab-switch case?
> All I can see is calls to _applyPrefToSetting which test for siteSpecific too.

Well, it's faster to early abort if we know we can, right?

> >+  Services.prefs.setCharPref("toolkit.zoomManager.zoomValues",
> >+                             zoomBundle.getString("values").split(",")
> >+                                       .map(function(aVal) {return aVal/100;})
> >+                                       .join(","));
> Why not use zoomFactors instead of splitting twice?

To actually be using the same things as toolkit does. And yes, I know, it's somewhat of a hack, but I didn't want to go and change the values in our menu. I should file a bug though to actually use the pref correctly and not hack it in this way (I'm glad I remember this somewhat, this work is months back).

> >+      if (item.getAttribute("value") == parseInt(ZoomManager.zoom * 100))
> Should we make it so that the value attribute is the zoom as a fraction?

I played around with those things somewhat when I worked on this, and IIRC found that this would be a good step once we switch to using the toolkit.zoomManager.zoomValues pref correctly, but probably ends up in more work as long as we're using the values from the bundle as we have been up to now.
In the end, IIRC, some accesskey hell made me decide to leave things with the values defined in the bundle for now.

(In reply to comment #26)
> >+  var menuLabel = zoomBundle.getString(zoomType).replace(/%zoom%/, parseInt(ZoomManager.zoom * 100));
> ...
> >+      if (item.getAttribute("value") == parseInt(ZoomManager.zoom * 100))
> These need to round. Otherwise 90% becomes 89% (89.99999976158142+).

Hmm, yes, I guess Math.round() is best here, we really want integers, any more precision is not too useful in this case.
Comment 29 Robert Kaiser (not working on stability any more) 2010-12-07 12:31:03 PST
Created attachment 495921 [details] [diff] [review]
v1.2: fix further review comments

OK, I hope this one fixes the remaining comments.
Comment 30 neil@parkwaycc.co.uk 2010-12-07 16:15:45 PST
(In reply to comment #28)
>(In reply to comment #25)
>>Finally you don't pass the browser to onLocationChange. I'm concerned that this
>>will cause problems if you switch tabs quickly. Can you not pass in browser
>>(conveniently defined earlier in onLocationChange)?
>I guess using onUpdateCurrentBrowser would have solved all those. :P
Well, going by your previous patch, I would have still asked you to pass in the browser (assuming onUpdateCurrentBrowser knows which one it is).

>>>+  onLocationChange: function FullZoom_onLocationChange(aURI, aIsTabSwitch, aBrowser) {
>>>+    if (!aURI || (aIsTabSwitch && !this.siteSpecific))
>>When site specific preferences are off, what happens in a non-tab-switch case?
>>All I can see is calls to _applyPrefToSetting which test for siteSpecific too.
>Well, it's faster to early abort if we know we can, right?
Sure, but why abort early only in the aIsTabSwitch case?

>>>+  Services.prefs.setCharPref("toolkit.zoomManager.zoomValues",
>>>+                             zoomBundle.getString("values").split(",")
>>>+                                       .map(function(aVal) {return aVal/100;})
>>>+                                       .join(","));
>>Why not use zoomFactors instead of splitting twice?
>To actually be using the same things as toolkit does. And yes, I know, it's
>somewhat of a hack, but I didn't want to go and change the values in our menu.
That wasn't actually my point. You assign zoomFactors to be zoomBundle.getString("values").split(",") later on in the function.
Comment 31 neil@parkwaycc.co.uk 2010-12-07 16:17:41 PST
(In reply to comment #28)
> Still, I'll just apply all those changes you are wishing for here and hope it
> works out.
The change looks right at first glance.
Comment 32 Robert Kaiser (not working on stability any more) 2010-12-09 08:25:46 PST
(In reply to comment #30)
> Sure, but why abort early only in the aIsTabSwitch case?

You're right. We don't do anything in that function that would be specific to this case only here. If it would not have some potential value for keeping FF compat, we could actually kill that parameter completely...
For reference, Firefox added it for private browsing in bug 487656 and their _applyPrefToSetting is actually executed when in private browsing mode even if .siteSpecific is off. If SeaMonkey ever implements private browsing, I hope whoever works on that will not forget this case.

> That wasn't actually my point. You assign zoomFactors to be
> zoomBundle.getString("values").split(",") later on in the function.

Ah, thanks, I somehow missed that. Fixed in a new patch.
Comment 33 Robert Kaiser (not working on stability any more) 2010-12-09 08:26:43 PST
Created attachment 496499 [details] [diff] [review]
v1.3: fixes for (hopefully) last two comments

Here's the patch with those two updates.
Comment 34 Robert Kaiser (not working on stability any more) 2010-12-09 08:58:51 PST
Thanks, pushed as http://hg.mozilla.org/comm-central/rev/61c697891f6c

Note You need to log in before you can comment on or make changes to this bug.