Closed Bug 814435 Opened 7 years ago Closed 6 years ago

Display scroll bars when async pan and zoom is turned on

Categories

(Core :: Panning and Zooming, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29
blocking-b2g 1.3+
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
b2g18 + wontfix
b2g18-v1.0.1 --- wontfix
b2g-v1.1hd --- wontfix
b2g-v1.2 --- wontfix
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: carlosmartinez, Assigned: kats)

References

Details

(Whiteboard: [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2, leorun3, leorun4, retest_leorun4, burirun1 )

Attachments

(5 files, 10 obsolete files)

5.65 KB, patch
Details | Diff | Splinter Review
11.24 KB, patch
botond
: review+
Details | Diff | Splinter Review
1.29 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
4.89 KB, patch
BenWa
: review+
Details | Diff | Splinter Review
15.11 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
Tested with unagi with:

gaia 620399e
gecko 85d0121

STR:
1-Open browser app
2-Surf to slashdot.org
3-Double tap to zoom
4-Scroll up & down and left & rigth

Expected result --> Scroll bars should appear in the right side and down side of the screen
Actual result --> No scroll bar is shown
blocking-basecamp: --- → ?
Were these intentionally removed when async pan and zoom was turned on, or is this a regression?
Hi Josh, can you confirm comment#1 from Ben? is it the new design or a regression? thanks
Flags: needinfo?(jcarpenter)
Renominate if needed
blocking-basecamp: ? → -
Keywords: qawanted
(In reply to Joe Cheng from comment #2)
> Hi Josh, can you confirm comment#1 from Ben? is it the new design or a
> regression? thanks

Scroll bars should appear. Per Ben, this sounds like either a result of async pan-zoom being turned on, or a regression.
Flags: needinfo?(jcarpenter)
Whiteboard: [Browser][TR 11-15-12] → [TR 11-15-12]
defect is still reproducing using Unagi, build 20130104070203
UCID: browser-000
https://moztrap.mozilla.org/results/case/61262/
Whiteboard: [TR 11-15-12] → [TR 11-15-12], testrun 2
Reproducible on the 2013-01-12 eng unagi build:
- build identifier 20130112063240
- git commit df38c1bb813029f3ccfa4a997fb1529b..
Keywords: qawanted
Repros on Unagi device Build ID: 20130115070201   using the december 5th kernel
I believe we should be tracking this bug for the future.  renoming for tracking b2g-18
blocking-basecamp: - → ---
tracking-b2g18: --- → ?
this still reproduces on:
Unagi Build ID: 20130225070200
Dec 5th Kernel
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/3a5a27992a75
Gaia: 5691a16fff8e1403c75ed9d6f3a443b7e58198c6
Whiteboard: [TR 11-15-12], testrun 2 → [TR 11-15-12], testrun 2, testrun 5.1
Whiteboard: [TR 11-15-12], testrun 2, testrun 5.1 → [TR 11-15-12], testrun 5.1
This still repros on Inari Build ID: 20130417070205
Kernel Date: Feb 21
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/6bac24e14538
Gaia: 2d048a9bdae54e4ec7d48326c2130591c8b869b6

When the user loads the URL of a long horizontal webpage and presses return to load the page,no vertical and/or horizontal scroll bars appear

UCID:Browser-000
Testcase: https://moztrap.mozilla.org/runtests/run/1105/env/316/?&pagenumber=1&pagesize=20&sortfield=order&sortdirection=asc&filter-id=1739
Whiteboard: [TR 11-15-12], testrun 5.1 → [TR 11-15-12], testrun 5.1, inarirun1
Whiteboard: [TR 11-15-12], testrun 5.1, inarirun1 → [TR 11-15-12], testrun 5.1, inarirun1, leorun1
If I use the facebook app I get scroll bars, if I load the same web site in the browser I don't. I'm guessing they're not enabled for iframes which are using APZC.
Component: Gaia::Browser → General
Summary: [Browser][TR 11-15-12] No vertical nor horizontal scroll bars appear in very long and/or wide web pages → Display scroll bars when async pan and zoom is turned on
Whiteboard: [TR 11-15-12], testrun 5.1, inarirun1, leorun1 → [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2
Duplicate of this bug: 881919
Whiteboard: [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2 → [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2, leorun3
What is the decision on this feature?  Do we want this or not - I can't ascertain that from the discussion here nor in the product requirements.

According to Josh's comment 4, scroll bars should appear.   To nom or not to nom?
Flags: needinfo?(pdolanjski)
Let's look at more data

slahdot.org enables/disables scrollbars, and has only a desktop sites according to different criteria, but IMO, mobile sites should probably have some kind a scrollbar by default.

Trying out most of the top sites according to Alexa.com, and not a single one  brings up any sort of scrollbar.  I'm inclined to renom this bug, but still waiting for clarification.
Moving NI to Chris and Francis, who are the Browser PM and Browser UX going forward.
Flags: needinfo?(pdolanjski) → needinfo?(fdjabri)
Please renominate this bug as this is something we definitely want for the browser from both a product and UX perspective.
Flags: needinfo?(fdjabri)
Duplicate of this bug: 888795
Whiteboard: [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2, leorun3 → [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2, leorun3, leorun4
Whiteboard: [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2, leorun3, leorun4 → [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2, leorun3, leorun4, retest_leorun4
According to Josh in Comment #4, scroll bars should appear.   So nomming for Koi.
blocking-b2g: --- → koi?
John, do you really believe that this is a blocker, as in if this was the last bug we had we would delay the 1.2 release for it?

We're trying to make "blocking" actually mean blocking in 1.2 and use the browser backlog for general prioritisation of work https://www.pivotaltracker.com/s/projects/867301
Flags: needinfo?(jhammink)
Happy to denom this, if it's the case that this is no longer a requirement (at one point, I believe it was, although searching through our requirements haystack for the original referring one is a bloody mess).

Thanks for the link on Pivotal tracker btw. 

Should we de-nom then and deactivate the regression testcase that keeps hitting this (for half a year now)?

I'll go ahead and do exactly this, let's pick this up later if we decide it matters.
blocking-b2g: koi? → ---
Flags: needinfo?(jhammink) → in-moztrap-
Test case 1739 has been disabled in moztrap per comment 21
Whiteboard: [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2, leorun3, leorun4, retest_leorun4 → [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2, leorun3, leorun4, retest_leorun4 burirun1
Whiteboard: [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2, leorun3, leorun4, retest_leorun4 burirun1 → [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2, leorun3, leorun4, retest_leorun4
Whiteboard: [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2, leorun3, leorun4, retest_leorun4 → [TR 11-15-12], testrun 5.1, inarirun1, leorun1, inarirun2, leorun3, leorun4, retest_leorun4, burirun1
Now that APZ is supposed to be used for all apps I want to leverage this issue. It seems bad to loose all scrollbars once async scroll will be the default behavior for apps.
Blocks: gaia-apzc
This wants an owner or it will turn into a long-pole.
This draws simple scroll bars for scrollable container layers. This is totally untested so far. Still missing: hiding the axis that can't scroll (if any), fade-out animation, rounded corners.
If anyone gets assigned to this, please grab my patches and continue and don't wait for me. I am on and off and traveling next week.
GetEffectiveTransform() seems to include the scroll transform. I need the transform of the layer rect without that. No idea how to do that.
Milan, I need some help here. I can't figure out what rect and transform to use. I don't think mCompositionBounds is necessarily correct, and EffectiveTransform includes the scroll offset. Kats probably can give some guidance here, or anyone else who knows the code.
Flags: needinfo?(milan)
Kats should be back today.  Vivien, I made this a 1.3 blocker, based on your comment 23, but if you think it should not be, please revert it back.
Assignee: nobody → bugmail.mozilla
blocking-b2g: --- → 1.3+
Flags: needinfo?(milan) → needinfo?(21)
So mCompositionRect is probably the right rect to use here. Its the size of the scrollable region in screen pixels, independent of zoom. Its (0,0) based, even when the url bar comes in, so the transform is what moves it into the right place.
I added ScreenPoint mScrollOffset; to LayerComposite to store the scroll offset we calculate in SampleContentTransformForFrame. That scroll offset is (0,0) based but definitely resolution dependent.
(In reply to Milan Sreckovic [:milan] from comment #31)
> Kats should be back today.  Vivien, I made this a 1.3 blocker, based on your
> comment 23, but if you think it should not be, please revert it back.

I think it should be, thanks Milan.
Flags: needinfo?(21)
Component: General → Panning and Zooming
Product: Firefox OS → Core
Its not a temporary quick jump. Once wrong the bars hang out in the wrong position.
Sorry kats, all you from here on. I will poke a bit but unlikely I will figure this out.
Flags: needinfo?(bugmail.mozilla)
I can reproduce the scrollbars ending up in the center. STR:

1. Apply patch from this bug
2. Start B2G browser and go to people.mozilla.com/~kgupta/grid.html (or any other page really)
3. Zoom out and wait for the repaint

This is happening because the composition bounds are incorrect when zoomed out; this is a known issue tracked in bug 935219. I will take a crack at that today to see if I can fix it up.
Depends on: 935219
Flags: needinfo?(bugmail.mozilla)
Botond is continuing to work on bug 935219. In the meantime I took a quick look at the pre-APZC scrollbars in gaia apps. Vivien pointed to me to where they are turned on and styled so I was able to turn them off. This results in one set of scrollbars rather than two, but also breaks scrolling unless we back out bug 825692. The attached patch does that. To summarize the various behaviours (with APZC enabled):

* Current gaia master: scrollbars are drawn by layout and so don't update async. Input fields are not scrollable.

* Gaia master + andreas' patch: two sets of scrollbars are drawn, one from layout and one in the compositor. Input fields are not scrollable.

* Gaia master + andreas' patch + attached patch: scrollbars are drawn in the compositor. Input fields are layerized and scrollable. Input fields also have scrollbars.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #39)
> Input fields also
> have scrollbars.

This artifact is making me think that perhaps drawing the scrollbars entirely in the compositor may not be the best idea. That is, if we have cases (such as input fields) which should be scrollable but not display scrollbars, and we are drawing the scrollbars entirely in the compositor, then we need to annotate the layers as to whether or not they should display scrollbars. Plus, all the styling that we can currently do with scrollbars no longer works.

Another approach might be to have the scrollbars generated in layout as they are now, but force them onto their own layers and then apply an additional transform to them in the compositor based on the async transform in the APZC. Layerizing the scrollbars (assuming they're not already) will make them more expensive but we might want them layerized anyway to have them fade in and out smoothly.
Filed bug 946408 for the input fields not being scrollable.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #40)
> 
> Another approach might be to have the scrollbars generated in layout as they
> are now, but force them onto their own layers and then apply an additional
> transform to them in the compositor based on the async transform in the
> APZC. Layerizing the scrollbars (assuming they're not already) will make
> them more expensive but we might want them layerized anyway to have them
> fade in and out smoothly.

I think scrollbars has been moved into their own layer in https://bug813041.bugzilla.mozilla.org/attachment.cgi?id=828520
We will never get proper responsiveness for scrollbars if we don't draw them entirely where the decisions on scrolling are made (which is the compositor).
Annotating scrollable layers that aren't supposed to have scroll bars is not difficult I think. We need a quick plan here. I don't really care about the precise approach but we have 4 days left and we are making zero progress here.
Attachment #8343164 - Flags: feedback?(tnikkel)
Attachment #8343164 - Flags: feedback?(bgirard)
Attachment #8341328 - Attachment description: Much better but through zooming and panning around and running into edge I can produce a condition where suddenly the scrollbars are more in the center. Looks like a race condition. → Alternative approach (scrollbars drawn entirely in compositor)
With these three patches the scrollbars in Gaia work as before, and track async panning as well. The only problem that I can see is that the fade animation is still based on when gecko repaints rather than the async panning. That is, the scrollbars will start to fade out while panning, and will un-fade when gecko repaints, which makes it seem random.
Comment on attachment 8343166 [details] [diff] [review]
Part 3 - Set a shadow transform on the scrollbar layer

Review of attachment 8343166 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/composite/AsyncCompositionManager.cpp
@@ +569,5 @@
> +                      1);
> +      transform.ScalePost(1.0f/aLayer->GetPostXScale(),
> +                          1.0f/aLayer->GetPostYScale(),
> +                          1);
> +      aLayer->AsLayerComposite()->SetShadowTransform(transform);

Oh, I need a break statement here, so that it only uses the first scrollTarget. Will fix locally.
(Added the break statement)
Attachment #8343166 - Attachment is obsolete: true
Attachment #8343166 - Flags: feedback?(bgirard)
Attachment #8343188 - Flags: feedback?(bgirard)
Comment on attachment 8343190 [details] [diff] [review]
Part 4 - Prevent the scrollbar fading out while user is scrolling

I can't say I like that but I can live with it for this deadline. Could we reduce a bit the 500ms delay just it does not seems like it takes forever ?
Attachment #8343190 - Flags: feedback?(21) → feedback+
Sure, I can reduce it. 500ms is what we use on Fennec and it seems to work well so I thought it would be fine here. 300ms should be fine too.
If we have scrollbars in both directions are we creating one big layer with mostly transparent pixels?
Attachment #8343164 - Flags: feedback?(tnikkel) → feedback+
Comment on attachment 8343165 [details] [diff] [review]
Part 2 - Always layerize overlay scrollbars

Do we want to limit the scope of this change? Only certain platforms or where it's needed maybe?
Attachment #8343165 - Flags: feedback?(tnikkel) → feedback+
(In reply to Andreas Gal :gal from comment #54)
> If we have scrollbars in both directions are we creating one big layer with
> mostly transparent pixels?

No, we should end up with separate layers for the vertical and horizontal scrollbars.

(In reply to Timothy Nikkel (:tn) from comment #55)
> Do we want to limit the scope of this change? Only certain platforms or
> where it's needed maybe?

Yeah. I can ifdef it for B2G but I was hoping you'd have suggestions on a more appropriate condition to use. Should I check for usingDisplayPort? That should identify only the scroll frames that have an APZC although it seems like checking for a side-effect that may happen in other scenarios too.
No longer need the dependency if we go with the new patch set.
No longer depends on: 935219
Attachment #8343164 - Flags: review?(tnikkel)
Attachment #8343164 - Flags: review?(bgirard)
Attachment #8343164 - Flags: feedback?(bgirard)
Attachment #8343188 - Flags: feedback?(bgirard) → review?(bgirard)
I found the right way to do this. Turned out to be pretty straightforward so I hooked it up.
Attachment #8343190 - Attachment is obsolete: true
Attachment #8343772 - Flags: review?(botond)
Comment on attachment 8343772 [details] [diff] [review]
Part 4 - Prevent the scrollbar fading out while user is scrolling

Review of attachment 8343772 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. One comment: is there a reason the GeckoContentController interface takes a full ScrollableLayerGuid but we only pass the scroll id over PBrowser? If not, I'd prefer the two to be consistent.
Attachment #8343772 - Flags: review?(botond) → review+
Attachment #8343164 - Flags: review?(tnikkel) → review+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #56)
> (In reply to Timothy Nikkel (:tn) from comment #55)
> > Do we want to limit the scope of this change? Only certain platforms or
> > where it's needed maybe?
> 
> Yeah. I can ifdef it for B2G but I was hoping you'd have suggestions on a
> more appropriate condition to use. Should I check for usingDisplayPort? That
> should identify only the scroll frames that have an APZC although it seems
> like checking for a side-effect that may happen in other scenarios too.

I'm not sure off the top of my head. B2G only for now sounds okay until we figure out a better idea.
(In reply to Botond Ballo [:botond] from comment #59)
> Looks good. One comment: is there a reason the GeckoContentController
> interface takes a full ScrollableLayerGuid but we only pass the scroll id
> over PBrowser? If not, I'd prefer the two to be consistent.

At the level of the GeckoContentController interface, what the notification will be used for is unknown, and so it makes sense to have it take a ScrollableLayerGuid. That way the implementor knows exactly which APZC instance is being transformed, and they can do whatever they need to. I originally passed the ScrollableLayerGuid across the PBrowser interface too but then I realized that no other function does that, and in fact the layers id is meaningless in TabChild. Since it didn't make sense to pass the layers id I also took out the presshell id because it wasn't being used. I can put these back but it seems unnecessary to me, as this is really more of an implementation detail than a real cross-module interface.
Attachment #8343839 - Flags: review?(tnikkel) → review+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #61)
> (In reply to Botond Ballo [:botond] from comment #59)
> > Looks good. One comment: is there a reason the GeckoContentController
> > interface takes a full ScrollableLayerGuid but we only pass the scroll id
> > over PBrowser? If not, I'd prefer the two to be consistent.
> 
> At the level of the GeckoContentController interface, what the notification
> will be used for is unknown, and so it makes sense to have it take a
> ScrollableLayerGuid. That way the implementor knows exactly which APZC
> instance is being transformed, and they can do whatever they need to. I
> originally passed the ScrollableLayerGuid across the PBrowser interface too
> but then I realized that no other function does that, and in fact the layers
> id is meaningless in TabChild. Since it didn't make sense to pass the layers
> id I also took out the presshell id because it wasn't being used. I can put
> these back but it seems unnecessary to me, as this is really more of an
> implementation detail than a real cross-module interface.

That sounds reasonable, let's keep it as it is then.

(As we discussed on IRC, technically passing in the layers id to GeckoContentController is redundant as there is already a separate GeckoContentController instance for each layers id, but we can pass a ScrollableLayerGuid for convenience.)
Attachment #8343164 - Flags: review?(bgirard) → review+
Comment on attachment 8343188 [details] [diff] [review]
Part 3 - Set a shadow transform on the scrollbar layer

Review of attachment 8343188 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/composite/AsyncCompositionManager.cpp
@@ +530,5 @@
> +    // If this layer corresponds to a scrollbar, then search backwards through the
> +    // siblings until we find a container layer with an APZC. That should
> +    // (hopefully) be the content that this scrollbar is for. Pick up the
> +    // transient async transform from that layer and use it to update the
> +    // scrollbar position.

I'm a bit unsure about this. We're assuming that if we have a scrollbar layer then we have a layer for the scrollable content. I don't know if this is true and if there aren't race conditions (i.e. the scroll layer goes inactive before the scrollbar because we're fading out the scrollbar).

Can you convince me?
(In reply to Benoit Girard (:BenWa) from comment #64)
> I'm a bit unsure about this. We're assuming that if we have a scrollbar
> layer then we have a layer for the scrollable content. I don't know if this
> is true and if there aren't race conditions (i.e. the scroll layer goes
> inactive before the scrollbar because we're fading out the scrollbar).
> 
> Can you convince me?

If the layer for the scrollable content disappears then that means that we can't be asynchronously scrolling it, so we don't need to apply any async transform to the scrollbars. We can just leave them wherever Gecko painted them because that will be correct.

The only problematic scenario I can think of is if that loop actually finds some *other* scrollable layer that happens to be a sibling of the scroll target layer. I don't know if that can happen in practice and if it does, how often. I imagine that it will not be a problem in practice. If it is a problem we can probably deal with it by adding the ViewID onto the scrollbar layers in layout, and then match those with the ViewID on the scroll target.

Thoughts?
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #65)

> The only problematic scenario I can think of is if that loop actually finds
> some *other* scrollable layer that happens to be a sibling of the scroll
> target layer.

It's the type of bugs I'm worried about. Where we match another layer.

> I don't know if that can happen in practice and if it does,
> how often. I imagine that it will not be a problem in practice. If it is a
> problem we can probably deal with it by adding the ViewID onto the scrollbar
> layers in layout, and then match those with the ViewID on the scroll target.
> 
> Thoughts?

For me this sounds like your admitting that it can happen and its things like that which cause flaky code which APZC already has a ton of. IMO we should deal with this now.
Fair enough. I've updated part 1 to also propagate the ViewID. I changed how the data was stored on the layer since I couldn't stuff it into the flags any more. Mostly just copied what the sticky position data was doing. Re-requesting review from tn for some minor changes to the layout bits as well.
Attachment #8343164 - Attachment is obsolete: true
Attachment #8343978 - Flags: review?(tnikkel)
Attachment #8343978 - Flags: review?(bgirard)
Attachment #8343978 - Attachment description: Part 1 - Annotate layers for scrollbars → Part 1 - Annotate layers for scrollbars (v2)
Updated to ensure it finds the right target layer.
Attachment #8343188 - Attachment is obsolete: true
Attachment #8343188 - Flags: review?(bgirard)
Attachment #8343979 - Flags: review?(bgirard)
Comment on attachment 8343978 [details] [diff] [review]
Part 1 - Annotate layers for scrollbars (v2)

Review of attachment 8343978 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/Layers.h
@@ +1398,5 @@
> +  struct ScrollbarData {
> +    FrameMetrics::ViewID mScrollId;
> +    ScrollDirection mDirection;
> +  };
> +  nsAutoPtr<ScrollbarData> mScrollbarData;

This structure is really small. It's probably smaller then a nsAutoPtr or nearly. For small structs we should avoid heap allocations IMO.

If you disagree at least add MOZ_COUNT_CTOR.
Attachment #8343978 - Flags: review?(bgirard) → review+
Updated to not use the nsAutoPtr struct and just have a mIsScrollbar bool instead. Carrying BenWa's r+ and tn's r?.
Attachment #8343978 - Attachment is obsolete: true
Attachment #8343978 - Flags: review?(tnikkel)
Attachment #8344321 - Flags: review?(tnikkel)
Comment on attachment 8344321 [details] [diff] [review]
Part 1 - Annotate layers for scrollbars (v3)

nsDisplayOwnLayer turning into a catch all for miscellaneous things starts to feel like the wrong way, but we can clean it up the next time we need to add to it, no need to hold this back from landing.
Attachment #8344321 - Flags: review?(tnikkel) → review+
Comment on attachment 8343979 [details] [diff] [review]
Part 3 - Set a shadow transform on the scrollbar layer (v2)

Review of attachment 8343979 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/composite/AsyncCompositionManager.cpp
@@ +524,5 @@
>      appliedTransform = true;
>    }
>  
> +  if (aLayer->GetIsScrollbar()) {
> +    // If this layer corresponds to a scrollbar, then search backwards through the

Optional:
Consider making the contents of this if() into it's own function. It has a well defined purpose and this function is already to long.
Attachment #8343979 - Flags: review?(bgirard) → review+
(In reply to Benoit Girard (:BenWa) from comment #72)
> Optional:
> Consider making the contents of this if() into it's own function. It has a
> well defined purpose and this function is already to long.

I did this. Also I forgot to initialize mIsScrollbar in the Layer constructor in part 1 so I did that.

Try push at https://tbpl.mozilla.org/?tree=Try&rev=f29bc60b87b2, will land if it looks good.
You need to log in before you can comment on or make changes to this bug.