Homescreen takes long to render after exiting an app

RESOLVED FIXED in Firefox 19

Status

defect
P1
normal
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: pla, Assigned: gsvelto)

Tracking

(Depends on 1 bug, {perf, ux-trust})

unspecified
B2G C3 (12dec-1jan)
x86
macOS
Dependency tree / graph

Firefox Tracking Flags

(blocking-basecamp:+, firefox19 fixed, firefox20 fixed, b2g18 fixed)

Details

(Whiteboard: interaction, UX-P1)

Attachments

(2 attachments, 6 obsolete attachments)

502.43 KB, video/mp4
Details
12.82 KB, patch
justin.lebar+bug
: review+
Details | Diff | Splinter Review
What makes it feel slow/broken?

After exiting any application, I go to a blank homescreen (just see the wallpaper), and it takes anywhere from 1-3 seconds for the dock and icons to appear.

Did it prevent you from doing what you wanted? Why?

It gave me some uncertainty as to what was going on.  I expect the homescreen to appear instantly, so if it doesn't, I might wonder if the OS has crashed.  Otherwise, it is just a nuisance because it prevents me from getting around the OS in a quick manner.

How does this make you feel?

[ ]  :)  I feel happy about it
[ ]  :|  Meh
[X]  :(  I'm upset
[ ] >:O  I'm angry

Device: Unagi, Nov. 22 Nightly.

Details:

It takes 1-3 seconds before the homescreen renders.  In the meantime, only the wallpaper is being displayed.  On our reference platform (Otoro running ICS 4.0.4), the homescreen renders instantly after exiting an app.

Bonus: can you attach a video of the problem?
Yes
Additional Comments.

The homescreen also DOES NOT have a proper transition when closing an app.  Please refer to page 6 ('App Closing') of the transitions document here: https://www.dropbox.com/s/5xbazzqs8i9vnji/Transitions_v10.pdf

The transition back to homscreen should animation smoothly.  Currently, when it is slow, it just appears in piecemeal fashion, one component at a time.
Priority: -- → P1
Summary: [Gaia::Homecreen][perf][uxtrust] Homescreen takes long to render after exiting an app → [Gaia::Homecreen][perf][ux-trust] Homescreen takes long to render after exiting an app
Whiteboard: perf, uxtrust → perf, ux-trust
Keywords: ux-trust
Summary: [Gaia::Homecreen][perf][ux-trust] Homescreen takes long to render after exiting an app → [Gaia::Homecreen] Homescreen takes long to render after exiting an app
Nominating for blocking-basecamp+.

This delay of up to several seconds occurs virtually every time the user exits to the homescreen, and looks broken.
blocking-basecamp: --- → ?
blocking-basecamp: ? → +
Target Milestone: --- → B2G C2 (20nov-10dec)
Target Milestone: B2G C2 (20nov-10dec) → ---
Bug 814322 will change the way this works and eliminate the jank.  It won't make the homescreen repaint faster though.
Depends on: 814322
Gabriele please, could you take a look and profile this ?
Flags: needinfo?(gsvelto)
I have a little bit of trouble reproducing this. If the phone is idle, or mostly idle, tapping the homescreen button gets me instantly to the homescreen; the background and icons are present just as I left them. On the other hand if the phone is already under load I can see the behavior described in comment 1. First of all I see the wallpaper and then the page and icons appear (after a while). A way to reproduce this is to point the browser to a page with an expensive long-running script and then going back to the homescreen. This happens also if the browser is busy in the background and I go to the homescreen from another app.

All in all it looks like the homescreen is being repainted when the user taps the home button and if the phone's fast enough all goes smoothly, if it isn't then one can temporarily see the background. I will try to capture a profile trace of the Homescreen app while this is happening to get a better idea of how it happens.
Flags: needinfo?(gsvelto)
I successfully narrowed down the set of conditions under which this problem happens and figured out what is causing it. When we go back to the homescreen the Homescreen application gets repainted. This happens normally fast enough that the user won't notice it, it will be hidden behind the foreground application window before it starts the shrinking transition.

However if the phone is under load the transition will take considerably longer. This is because when the homescreen app is sent to the background it's priority is greatly reduced and thus it's allocated less CPU time. This can cause the homescreen wallpaper to briefly show up before the homescreen is reduced and is the first part of this problem.

However what's making the problem worse and causes the very long delay which can be sometimes experienced is a piece of functionality we added in bug 800166. When an application is sent into the background we force it to minimize its memory usage but this does not happen right away; the memory minimization procedure is just scheduled to run at the next possible time. Since the homescreen has been sent into the background and it's priority has been lowered this procedure won't run for a while if the phone is busy with other apps. Under the worst conditions it will run only when the Homescreen is brought up again and will introduce a long delay before it can repaint.

To highlight the problem I've prepared two profiler traces, the first one shows the Homescreen repainting when it's brought up and the phone is mostly idle. Repainting the Homescreen takes ~200ms, the app is active for ~500 ms in total:

http://people.mozilla.com/~bgirard/cleopatra/#report=9dee118fb5bdaf9e43c0cf25795d5d4436b65300

In this second trace the phone is under heavy load with the browser app taking 90% CPU time. The repaint costs now ~450ms but it happens only after the memory minimization procedure which takes ~800ms (you can see three GC/CC cycles). The homescreen app is now active for over 1.5s in total.

http://people.mozilla.com/~bgirard/cleopatra/#report=37c6d93329b027e1c5eb32e83e9bcf92d25bd967

Because the phone was busy the whole procedure is also likely to have started later; unfortunately this cannot be measured with the profiler. Still I'm pretty positive it's happening and it's adding to the delay.

As an additional test I turned off the functionality added in bug 800166 and the problem went away almost completely. It would then happen only under a very heavy load and only for a fraction of a second.
So is the solution to cancel the minimize-memory-usage task if the app comes into the foreground?  That shouldn't be hard.
Whiteboard: perf, ux-trust → interaction
Based on the discussion, this is not a Gaia bug but a Gecko bug, right?

We can met UX requirement first by make sure the nextpaint takes < 100ms most of the time in this bug. Then, we can make the transition happen even quicker by implementing the nextpaint event.
Component: Gaia::Homescreen → General
(In reply to Justin Lebar [:jlebar] from comment #8)
> So is the solution to cancel the minimize-memory-usage task if the app comes
> into the foreground?  That shouldn't be hard.

That should solve the problem but I've been asking myself when the Homescreen app is considered to be running in the foreground again. Is it when the user taps the home button or only after the application shrinking transition has finished? In the latter case we would cancel the minimize-memory-usage task too late. Anyway as you say it shouldn't be hard to test, I can try it today and see if it improves the situation.
(In reply to Gabriele Svelto [:gsvelto] from comment #10)
> That should solve the problem but I've been asking myself when the
> Homescreen app is considered to be running in the foreground again. Is it
> when the user taps the home button or only after the application shrinking
> transition has finished? In the latter case we would cancel the
> minimize-memory-usage task too late. Anyway as you say it shouldn't be hard
> to test, I can try it today and see if it improves the situation.

With bug 808231 we'd set the homescreen visible, wait for the next paint and then start the transition. Both of these bugs combined should give a good result, then.
Posted patch WIP patch (obsolete) — Splinter Review
I've prepared a simple patch that adds a method to nsMemoryReporterManager used to cancel the GC/CC cycles, this then gets called when the app goes into the foreground.

It can probably be simplified further by adding a static method that clears MinimizeMemoryUsageRunnable::mRemainingIters and getting rid of the weak reference to the object I'm currently using. However I want to test it first to see if it works and produces the desired effect.
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
This patch cancels a pending memory minimization procedure by keeping around a weak pointer to the MinimizeMemoryRunnable once it has been dispatched to the main thread and by setting to 0 the number of iterations it still needs to run. I hoped to make the patch simpler but making the weak references work correctly ended up making it larger than I had anticipated.

I've tested the patch on my Otoro and it does succeed in canceling pending minimizations however it does not completely solve the problem. The time during which the wallpaper shows up is shortened and in general it seems to be happening less often.

However since it's impossible to cancel a GC/CC cycle that is already running this is not a complete solution to the problem. I've noticed that if the first GC/CC collection gets to run and I go back to the homescreen I can still see the wallpaper showing up under load, even though the two following GC/CC cycles won't be executed. We should probably block on bug 808231 for a complete solution to this problem.
Attachment #688336 - Attachment is obsolete: true
Attachment #688817 - Flags: feedback?(justin.lebar+bug)
I don't think this is a good interface, because it allows one consumer to accidentally step on another consumer's toes.

As a hypothetical example, suppose we loaded about:memory on the phone (you can do this on Android), and the minimize-memory-usage button.  Then we suddenly decided the process was foreground, for whatever reason, and we canceled the minimize operation.

What if we had nsIMemoryReporterManager return an nsICancelableRunnable (a new interface inheriting from nsIRunnable)?
Attachment #688817 - Flags: feedback?(justin.lebar+bug) → feedback+
Component: General → Gaia::Homescreen
Summary: [Gaia::Homecreen] Homescreen takes long to render after exiting an app → Homescreen takes long to render after exiting an app
Whiteboard: interaction → interaction, UX-P1
(In reply to Justin Lebar [:jlebar] from comment #14)
> I don't think this is a good interface, because it allows one consumer to
> accidentally step on another consumer's toes.
>
> As a hypothetical example, suppose we loaded about:memory on the phone (you
> can do this on Android), and the minimize-memory-usage button.  Then we
> suddenly decided the process was foreground, for whatever reason, and we
> canceled the minimize operation.

Right, I hadn't thought of that. I've updated the patch with a simple solution to the problem. MinimizeMemoryUsage() now has a parameter that specifies if the operation can be canceled or not. Additionally if more operations are specified back-to-back they will all be executed while in the previous patch new operations would be ignored until the previous one had executed.

> What if we had nsIMemoryReporterManager return an nsICancelableRunnable (a
> new interface inheriting from nsIRunnable)?

That sounds better, in this case the consumer would want to hold a (weak) reference to the runnable and invoke a cancel function on its own. I'll prepare another patch in addition to this one so we can compare the two approaches.
Attachment #688817 - Attachment is obsolete: true
Attachment #689159 - Flags: feedback?(justin.lebar+bug)
Comment on attachment 689159 [details] [diff] [review]
[PATCH v2] Cancel a pending memory minimization procedure when an application is brought to the foreground

Okay, I'll wait for feedback until we have the other patch.
Attachment #689159 - Flags: feedback?(justin.lebar+bug)
This is identical to the previous patch but with the UUID for the nsIMemoryReporterManager changed because its interface had been altered.
Attachment #689159 - Attachment is obsolete: true
This is the alternative patch that adds an nsICancelableRunnable interface (plus helper in nsThreadUtils.(h|cpp) and makes nsIMemoryReporterManager return an instance of it when launching the memory minimization task. As with the other patch we keep track only of the latest task so if two are submitted back-to-back we won't be able to cancel the former.

I took the liberty of adding a bit of javadoc/doxygen documentation because I discovered that somebody's actually using these tools to extract documentation from the code, see https://developer.mozilla.org/en-US/docs/Developer_Guide/Interface_development_guide/Commenting_IDL_for_better_documentation .
Attachment #689805 - Flags: feedback?(justin.lebar+bug)
Comment on attachment 689805 [details] [diff] [review]
[PATCH] Add a cancelable runnable and use it to cancel a pending memory minimization procedure when an application is brought to the foreground

>diff --git a/xpcom/threads/nsICancelableRunnable.idl b/xpcom/threads/nsICancelableRunnable.idl
>new file mode 100644
>--- /dev/null
>+++ b/xpcom/threads/nsICancelableRunnable.idl

>@@ -0,0 +1,20 @@
>+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
>+/* This Source Code Form is subject to the terms of the Mozilla Public
>+ * License, v. 2.0. If a copy of the MPL was not distributed with this
>+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>+
>+#include "nsIRunnable.idl"
>+
>+/**
>+ * Represents a task which can be dispatched to a thread for execution and
>+ * which can be cancelled if necessary.
>+ */
>+
>+[scriptable, function, uuid(de93dc4c-5eea-4eb7-b6d1-dbf1e0cef65c)]

Hmmm...I'm not sure that [function] is right.  I think that makes it so that if
you do

  var foo = [some nsICancelableRunnable]
  foo();

that will run the one method in nsICancelableRunnable, cancel().  At least,
that's what I expect to happen.

>+interface nsICancelableRunnable : nsIRunnable
>+{
>+    /**
>+     * Cancels the task before it is run.

What happens if I cancel the task after it's finished running?  Does this
method return an error?  If so, which one?  What about if I cancel a task
twice?

>+     */
>+    void cancel();
>+};

>@@ -305,17 +313,20 @@
> 
>   mGracePeriodTimer = nullptr;
>   hal::SetProcessPriority(getpid(), PROCESS_PRIORITY_BACKGROUND);
> 
>   // We're in the background; dump as much memory as we can.
>   nsCOMPtr<nsIMemoryReporterManager> mgr =
>     do_GetService("@mozilla.org/memory-reporter-manager;1");
>   if (mgr) {
>-    mgr->MinimizeMemoryUsage(/* callback = */ nullptr);
>+    nsCOMPtr<nsICancelableRunnable> runnable;
>+    mgr->MinimizeMemoryUsage(/* callback = */ nullptr,
>+                             getter_AddRefs(runnable));
>+    mMemoryMinimizerRunnable = do_GetWeakReference(runnable);

Perhaps you want to cancel the old mMemoryMinimizerRunnable here.

>diff --git a/xpcom/base/nsMemoryReporterManager.cpp b/xpcom/base/nsMemoryReporterManager.cpp
>--- a/xpcom/base/nsMemoryReporterManager.cpp
>+++ b/xpcom/base/nsMemoryReporterManager.cpp

>@@ -899,33 +899,46 @@
>     os->NotifyObservers(nullptr, "memory-pressure",
>                         NS_LITERAL_STRING("heap-minimize").get());
>     mRemainingIters--;
>     NS_DispatchToMainThread(this);
> 
>     return NS_OK;
>   }
> 
>+  NS_IMETHOD Cancel()
>+  {
>+    mRemainingIters = 0;
>+
>+    return NS_OK;

For API cleanliness, I'd expect we'd want Cancel() to throw if the runnable has
already finished or already been canceled, so we can tell the difference
between these cases.  In which case you'd want to change this.

We probably want it to be an error if we call Run() twice, or if we call Run()
after Cancel().

Also, setting mRemainingIters = 0 doesn't always cancel this event: If we've
already done three minimize memory usage calls and scheduled our final run,
we'll still do the |if (mRemainingIters == 0)| case, which I guess we'd want to
avoid, since it can kick off slow operations?

> NS_IMETHODIMP
>-nsMemoryReporterManager::MinimizeMemoryUsage(nsIRunnable* aCallback)
>+nsMemoryReporterManager::MinimizeMemoryUsage(nsIRunnable* aCallback,
>+                                             nsICancelableRunnable **result)
> {
>-  nsRefPtr<MinimizeMemoryUsageRunnable> runnable =
>+  NS_ENSURE_ARG_POINTER(result);
>+
>+  nsRefPtr<nsICancelableRunnable> runnable =
>     new MinimizeMemoryUsageRunnable(aCallback);
>+  NS_ENSURE_TRUE(runnable, NS_ERROR_OUT_OF_MEMORY);

|new| is infallible in Gecko; you don't need to check the result, because it's
never NULL.  (Some old code still checks the result, because there was a time
when new was fallible.)  Note that malloc() is not infallible.

>+  NS_ADDREF(*result = runnable);
>+
>   return NS_DispatchToMainThread(runnable);
> }

This is totally the right idea, but I'd like to have another look before we check this in.
Attachment #689805 - Flags: feedback?(justin.lebar+bug) → feedback+
(In reply to Justin Lebar [:jlebar] from comment #19)
> Hmmm...I'm not sure that [function] is right.
> [snip]
> that will run the one method in nsICancelableRunnable, cancel().

Yes, you're right, my bad. I had copy-pasted nsIRunnable.idl and didn't pay attention to that attribute.

> What happens if I cancel the task after it's finished running?  Does this
> method return an error?  If so, which one?  What about if I cancel a task
> twice?

I really don't have a good answer for that question, maybe we might add an error or something but the trouble is that it's only an interface what we're providing so it's really up to the implementer to decide what to do. It would be great to offer a default behavior (like no-cancel after run, no double-cancel, etc...) but I haven't found a good way to do it lest adding a helper class with the right checks and having the user overload another method of it:

class nsSaneCancelableRunnable : nsICancelableRunnable {
  void Cancel() {
    if (already_run) {
      // No good
    } else if (already_canceled) {
      // Give an error
    } else {
      CancelForReal()
    }
  }

  CancelForReal() {
    // The user actually overrides this one and not Cancel()
  }
}

That doesn't look very elegant though and a user could still create a direct implementation of nsICancelableRunnable where Cancel() can be called many times, etc...

> Perhaps you want to cancel the old mMemoryMinimizerRunnable here.

Yes, makes sense.

> For API cleanliness, I'd expect we'd want Cancel() to throw if the runnable has
> already finished or already been canceled, so we can tell the difference
> between these cases.  In which case you'd want to change this.
>
> We probably want it to be an error if we call Run() twice, or if we call
> Run() after Cancel().

I thought a bit about solving these cases and I came to the conclusion that maybe doing it this way (only providing a nsICancelableRunnable interface) is not the right solution. First of all what we're doing here is adding a way to soft-cancel a runnable but we'd want something that guarantees cancellation. Since the runnable itself cannot take itself out of a queue, what it can do is set some kind of flag and then cancel itself when Run() is invoked the next time but that doesn't guarantee anything. Additionally the runnable also has no idea of what stage of execution it's in; some implementers like this one expect Run() to be called multiple times because they dispatch themselves from it so adding a specific behavior for cancel-after-run errors is tricky.

My feeling is that a better approach would be to add a cancel() method to nsIEventTarget which cancels pending runnables (and thus can return the proper error conditions); that way whoever implements the queue of runnables can hard-cancel one and make sure Run() is never called at all. I'd still like to add an nsICancelableRunnable() class for two reasons: we probably don't want to allow regular runnables to be canceled and we might want to let the user do some cleanup in Cancel(), or send an notification, etc... nsICancelableRunnable::cancel() would thus be called by nsIEventTarget::cancel() upon terminating the runnable.

Alternatively we could skip adding nsICancelableRunnable but adding NS_DISPATCH_CANCELABLE to nsIEventTarget::dispatch() flags instead.

> |new| is infallible in Gecko; you don't need to check the result, because it's
> never NULL.  (Some old code still checks the result, because there was a time
> when new was fallible.)  Note that malloc() is not infallible.

I see, I just tend to look at existing patterns in the code for guidance but from time to time they bring me down the wrong path :-(

> This is totally the right idea, but I'd like to have another look before we
> check this in.

OK, I'd rather wait a little bit more and come up with a more robust and serviceable interface than rushing it.
> but the trouble is that it's only an interface what we're providing so it's really up to the 
> implementer to decide what to do.

Sure.  I'd just like us to specify some behavior in the interface.  Then it's up to implementers to follow that contract.

I don't think we need anything more complex than this -- sure, it would be nice if we could implement Cancel() just once, but like you say, that's hard.

Sorry I wasn't more clear.
(In reply to Justin Lebar [:jlebar] from comment #21)
> Sure.  I'd just like us to specify some behavior in the interface.  Then
> it's up to implementers to follow that contract.
> 
> I don't think we need anything more complex than this -- sure, it would be
> nice if we could implement Cancel() just once, but like you say, that's hard.
> 
> Sorry I wasn't more clear.

I see, thanks for clearing it up. If you don't mind besides improving attachment 689805 [details] [diff] [review] with your suggestions I'd also like to cook up a patch of the alternative I proposed in my previous comment to gauge how complex it is. To me it just feels like a more robust and cleaner approach.

Alternatively we could use attachment 689803 [details] [diff] [review] to fix this specific problem and then open another bug for providing 'cancelable runnable' functionality which would also be used to convert this particular instance of it.
Given that basically nobody else is using a cancelable runnable, I don't think it's worth putting a ton of effort into a general solution here.  But that's just me; I'm happy with whatever approach you want to take.
The previous patch could allow the same runnable to be canceled twice because it the weak pointer wasn't cleared after the first cancel; this patch fixes the problem.
Attachment #689803 - Attachment is obsolete: true
Attachment #690937 - Flags: feedback?(justin.lebar+bug)
I've refreshed both patches fixing the remaining issues and experimented a bit with adding a cancel() method to nsIEventTarget. The latter approach requires non-trivial changes to nsEventQueue which sits in the core of Gecko and is used by every caller of nsIThread::Dispatch(). Since touching it is pretty risky without extensive tests I'd rather not do it now that we've got a tight schedule.

Instead I would say let's pick one of the two patches, polish it, push it through try and close this bug. Afterwards I'll open another bug for adding functionality to nsIEventTarget that would cover cancelable (and possibly run-when-idle) tasks. The idea would be to use that instead when there's enough time to implement it.
Comment on attachment 690943 [details] [diff] [review]
[PATCH v2] Add a cancelable runnable and use it to cancel a pending memory minimization procedure when an application is brought to the foreground

I'm happy with this if you are.
Attachment #690943 - Flags: feedback?(justin.lebar+bug) → review+
Comment on attachment 690937 [details] [diff] [review]
[PATCH v4] Cancel a pending memory minimization procedure when an application is brought to the foreground

I prefer the approach in the other patch.
Attachment #690937 - Flags: feedback?(justin.lebar+bug)
(In reply to Justin Lebar [:jlebar] from comment #27)
> Comment on attachment 690943 [details] [diff] [review]
> [PATCH v2] Add a cancelable runnable and use it to cancel a pending memory
> minimization procedure when an application is brought to the foreground
> 
> I'm happy with this if you are.

OK, off it goes to the try servers.
Try run for 5072118b1a59 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=5072118b1a59
Results (out of 15 total builds):
    success: 15
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/gsvelto@mozilla.com-5072118b1a59
Attachment #690937 - Attachment is obsolete: true
All green, ready for check-in.

Before this ticket gets closed however I'd like to strive the fact that my patch mitigates this problem but alone is not enough to resolve it completely. A full resolution requires what has been mentioned in comment 9 (waiting for nextpaint) which I think is being tracked in bug 808231.
Keywords: checkin-needed
(In reply to Gabriele Svelto [:gsvelto] from comment #31)
> Before this ticket gets closed however I'd like to strive the fact that my
> patch mitigates this problem but alone is not enough to resolve it
> completely. A full resolution requires what has been mentioned in comment 9
> (waiting for nextpaint) which I think is being tracked in bug 808231.

This has been merged to gaia-master.
https://hg.mozilla.org/mozilla-central/rev/6464b79a1cf9
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
I guess we want this to be merged to aurora and b2g beta as well?
Keywords: checkin-needed
(In reply to Gregor Wagner [:gwagner] from comment #35)
> I guess we want this to be merged to aurora and b2g beta as well?

Yes, but so long as the bug is blocking+, we don't need checkin-needed to get RyanVM's attention.
Keywords: checkin-needed
(In reply to Justin Lebar [:jlebar] from comment #36)
> (In reply to Gregor Wagner [:gwagner] from comment #35)
> > I guess we want this to be merged to aurora and b2g beta as well?
> 
> Yes, but so long as the bug is blocking+, we don't need checkin-needed to
> get RyanVM's attention.

We do. RyanVM only looks in the gecko component. For patches in gaia land we should use checkin-needed: https://bugzilla.mozilla.org/show_bug.cgi?id=818507#c21
Duly noted; thanks!
We might as well change the component, since we landed a Gecko change here...
Component: Gaia::Homescreen → General
https://hg.mozilla.org/releases/mozilla-aurora/rev/4739a79d8ab5

Doesn't apply on 18 but needs only minor corrections. Trying locally first before pushing.
Target Milestone: --- → B2G C3 (12dec-1jan)
Try run for 5072118b1a59 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=5072118b1a59
Results (out of 16 total builds):
    exception: 1
    success: 15
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/gsvelto@mozilla.com-5072118b1a59
You need to log in before you can comment on or make changes to this bug.