849399 - opacity sometimes leaps up to 1 (then back down) when it should be transitioning.

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[mozilla]

Attachment: Cut down test case that shows the issue, the logs "bef" and "aft" show the opacity jump from 0 to 1 and the glitch can be observed.

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130219185710

Steps to reproduce:

1. Add an opacity transition to a div.
2. Set the opacity to 0.
3. Set a "transitionend" listener and then set the opacity back to "1" when the opacity reaches 0.


Actual results:

Right before the opacity starts transitioning from 0 to 1, it briefly flickers up to 1.

Not only is this pretty ugly, it prevents scripts from determining if a transition is necessary.


Expected results:

The element should have faded out (as it did) then smoothly faded back in.

Comment 1

[mozilla]

So the log order in the attached test case reads:

bef 1
aft 1
bef 0
aft 1

However between the second "bef" and "aft" pair the node should have been transitioning, it shouldn't go directly from 0 to 1. On the screen at this point you can notice it flicker briefly. The opacity obviously goes back down though because after it flickers the div fades up again slowly.

Comment 2

[mozilla]

By altering the "setTimeout" delay the flicker can be prevented. Making it 50ms or higher seems to prevent the issue. The bug exists even with the setTimeout removed.

Comment 3

[mozilla]

Attachment: Should have used "one" rather than "bind" in test case, observed effects unchanged.

Comment 4

[(mostly gone) XtC4UaLL [:xtc4uall]]

Confirmed with:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0 ID:20130307023931 CSet: 6ffe3e9da8a8
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0 ID:20130307075451 CSet: a853b233420d
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130308 Firefox/21.0 ID:20130308042013 CSet: 1388e740e38b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130309 Firefox/22.0 ID:20130309030841 CSet: e6215e0357fa

WFM with:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20100101 Firefox/17.0 ID:20130307074601 CSet: 8f1bec301236 (ESR)

MSIE 10

Comment 5

[Loic]

This testcase hurts the eyes, it would be better to catch and log the event when opacity briefly flickers up to 1. :)

My regression range:
m-c
good=2012-07-31
bad=2012-08-01
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9d2a7a8ca1c7&tochange=582d4c67b3a7

My guess goes to suspected bug 755084.

Comment 6

[Nick Cameron [:nrc]]

I can't repro this in current nightly on Linux. I've tried with OpenGL and software layers. I've logged the opacity of the layer in question and never see the opacity bounce as described, nor can I see a visible glitch (although I've been staring at so many blue boxes I am seeing little purple stars).

Can I confirm that this is for Linux (not b2g or something). And with a standard setup (no OMTC, no OMTA)? Are there any special steps to repro? Or is this fixed in current nightly (v22) and we want a fix for older versions? (I notice the affected version is 17).

Comment 7

[Nick Cameron [:nrc]]

Also, I just noticed the 'sometimes' in the bug title. Is this intermittent? How often does it happen?

Comment 8

[Nick Cameron [:nrc]]

Attachment: log opacity of colour layers

Comment 9

[Nick Cameron [:nrc]]

Attachment: test case without jquery and no need to reload to repeat

Click the box to start the transition.

I tried this test case and the original one.

Comment 10

[Loic]

(In reply to Nick Cameron [:nrc] from comment #7)
> Also, I just noticed the 'sometimes' in the bug title. Is this intermittent?
> How often does it happen?

Yes it's intermittent. Refreshing the testcase over and over is not necessarily the best way to reproduce it, but switch to another tab, wait for a few secs, then back to the testcase and refresh a few times.

In your tescase, I don't see the animation of opacity. Is it missing?

Comment 11

[Nick Cameron [:nrc]]

(In reply to Loic from comment #10)
> (In reply to Nick Cameron [:nrc] from comment #7)
> > Also, I just noticed the 'sometimes' in the bug title. Is this intermittent?
> > How often does it happen?
> 
> Yes it's intermittent. Refreshing the testcase over and over is not
> necessarily the best way to reproduce it, but switch to another tab, wait
> for a few secs, then back to the testcase and refresh a few times.
> 
> In your tescase, I don't see the animation of opacity. Is it missing?

Thanks, I'll give that a go.

You have to click on the blue box to start the animation.

Comment 12

[mozilla]

Comment on attachment 724729 [details]
test case without jquery and no need to reload to repeat

This doesn't trigger the issue for me but just confirmed it still does on original test case.

Comment 13

[mozilla]

With respect to "sometimes," in my original test case it triggers 100% of the time, and in the newly uploaded attachment 0% of the time. I guess how often it triggers is dependant on the platform? Using Linux here.

Comment 14

[Nick Cameron [:nrc]]

could you explain to me what the jquery stuff is doing please? Or even better provide a test case without the jquery which reproduces the bug (no need for the logging)? There must be something about your specific test case which is triggering this bug. Thanks!

Comment 15

[mozilla]

I thought the jquery was very clear, it simply sets the CSS property, I was just trying to save a bunch of characters off of the test sorry I didn't realise you guys weren't familiar with jquery. I can provide a version that doesn't use jquery.

Comment 16

[mozilla]

Attachment: Improved version of original test case that triggers the bug 100% of the time (for me).

Comment 17

[mozilla]

The new case shows the opacity can jump in either direction, up or down. For me here in Firefox 19.0.1 and 19.0.2 I get a jump in both directions after the first transition. However only the jump up is visually noticeable, the jump down is logged but can not be seen.

Comment 18

[mozilla]

WRT your comment "no need for the logging" I would state that logging is categorically needed to display this issue as the "jump up/down" even when not visually noticeable is still a huge problem for scripts that test whether the css property has changed in order to know whether a transition is necessary or not. To only cover the cases that show up without logging would not be to tackle the issue correctly.

Comment 19

[mozilla]

The reason I rebind the "transitionend" listeners here (rather than re-using the single listener) is to mimic another script I found that needed to bind many listeners as it handled multiple transitions.

Could it be that the issue is triggered by the rebinding of the "transitionend" event as I believe this is the only significant difference now between our scripts (removing the setTimeout in mine still triggers the issue, but increasing the value of the timeout obviates it).

Comment 20

[mozilla]

My last comment is wrong sorry for the e-mail noise. When I put logging into your script I also notice the "jumping" it's just that the visual glitch isn't noticeable without mine and the timeout.

Comment 21

[mozilla]

Did some further investigation.

In each case an opacity transition happens on an element:

Firefox:
1. Set opacity on element which previously had no opacity override (set via .style.opacity) to 1 - result: immediately after element has opacity of 0 (rather than "" before it was set) and 10ms later it has transitioning opacity (between 0 and 1.)
2. Set opacity on element to 1 which previously had opacity overridden as 0 - result: immediately after opacity is 1 (rather than 0 as before), 10ms later has transitioning opacity (so 10ms it has jumped down from 1 and is on its journey back to 1).

Chrome/IE 10:
In both instances after setting the opacity to 1 immediately testing the opacity afterwards gives a "transitioning" opacity rather than 1.

The same results can be observed when setting the opacity from A to B, 0 and 1 were just used as examples.

IMO Chrome/IE behaviour is certainly preferable as it allows one to check whether they need to wait for "transitionend" or not to detect when the state takes effect. In Firefox the same effect can be achieved only by delaying the test in a setTimeout and the timeout delay must be sufficiently large.

Comment 22

[Nick Cameron [:nrc]]

(In reply to mozilla from comment #21)
> Did some further investigation.
> 
> In each case an opacity transition happens on an element:
> 
> Firefox:
> 1. Set opacity on element which previously had no opacity override (set via
> .style.opacity) to 1 - result: immediately after element has opacity of 0
> (rather than "" before it was set) and 10ms later it has transitioning
> opacity (between 0 and 1.)

This seems like correct behaviour to me.

> 2. Set opacity on element to 1 which previously had opacity overridden as 0
> - result: immediately after opacity is 1 (rather than 0 as before), 10ms
> later has transitioning opacity (so 10ms it has jumped down from 1 and is on
> its journey back to 1).

This is a bug. Sounds like we are not getting the animated opacity, but the non-animated opacity which should be hidden by the animated version (for animating elements we keep two versions of the styles around - an animated and non-animated one, the latter is set to the values at the end of the animations and should be hidden by the former whilst animating).

Comment 23

[Nick Cameron [:nrc]]

(In reply to mozilla from comment #20)
> My last comment is wrong sorry for the e-mail noise. When I put logging into
> your script I also notice the "jumping" it's just that the visual glitch
> isn't noticeable without mine and the timeout.

I could not repro this with my version of the test case. My JS looks like

<script>
        var block1 = document.getElementById('block1');

        block1.addEventListener('transitionend', function(ev) {
          if (ev.target.id != 'block1') return;
          var opacityBefore = block1.style.opacity
          block1.style.opacity = 1;
          var change = block1.style.opacity - opacityBefore
          if (Math.abs(change) > 0) {
            console.log("opacity change of", change)
            console.log("old ", opacityBefore)
            console.log("new ", block1.style.opacity)
          }
        }, false);

        block1.addEventListener('click', function(ev) {
          var opacityBefore = block1.style.opacity
          block1.style.opacity = '0';            
          var change = block1.style.opacity - opacityBefore
          if (Math.abs(change) > 0) {
            console.log("opacity change of ", change)
            console.log("old ", opacityBefore)
            console.log("new ", block1.style.opacity)
          }
        }, false);
    </script>

Did you use some different logging?

Comment 24

[mozilla]

Why not just use the existing test cases?

Comment 25

[Nick Cameron [:nrc]]

(In reply to mozilla from comment #24)
> Why not just use the existing test cases?

I'm trying to figure out exactly what is causing the bug we're seeing. I don't feel that I have a minimal test case yet - the bug could be due to the timeout, something in the animation code, something in the code which extracts the style value to JS, something completely different. It is more difficult to find out what is going on if I can't see what you are describing with the browser in a debugger, so I am trying to find out exactly what you mean by "put logging into your script", because you are seeing different results from what I would interpret that to mean.

Comment 26

[Nick Cameron [:nrc]]

(In reply to Nick Cameron [:nrc] from comment #22)
> (In reply to mozilla from comment #21)
> > Did some further investigation.
> > 
> > In each case an opacity transition happens on an element:
> > 
> > Firefox:
> > 1. Set opacity on element which previously had no opacity override (set via
> > .style.opacity) to 1 - result: immediately after element has opacity of 0
> > (rather than "" before it was set) and 10ms later it has transitioning
> > opacity (between 0 and 1.)
> 
> This seems like correct behaviour to me.
> 
> > 2. Set opacity on element to 1 which previously had opacity overridden as 0
> > - result: immediately after opacity is 1 (rather than 0 as before), 10ms
> > later has transitioning opacity (so 10ms it has jumped down from 1 and is on
> > its journey back to 1).
> 
> This is a bug. Sounds like we are not getting the animated opacity, but the
> non-animated opacity which should be hidden by the animated version (for
> animating elements we keep two versions of the styles around - an animated
> and non-animated one, the latter is set to the values at the end of the
> animations and should be hidden by the former whilst animating).

I was wrong, this is not a bug. The logging here will always give show the specified value of opacity. To get the computed value, we must use |getComputedValue(node, "").opacity| which gives the expected value.

Comment 27

[Nick Cameron [:nrc]]

OK, so I can repro the visible flash with current Beta, but not current nightly (using [e-mail]'s test case). And not at all with mine. Logging the opacity using my patch also shows no jump in opacity. The JS logging is due to using the specified, rather than computed style. Therefore, I don't think there is a bug to fix and I propose we close this as WORKSFORME, unless anyone can actually observe the visible flash in current nightly. Loic? [e-mail]?

Comment 28

[mozilla]

Thanks for the info. Glad the visual flash has been fixed in the current nightly and I'd also like to point out you raise an interesting point. If getComputedStyle is necessary to get the actual style, I believe that setting the opacity from '' to 1 should return "1" rather than the transitioning opacity that it returns currently  (it gives "0"). Or maybe this has also been fixed in the nightly?

I guess now I have to raise bugs against Chrome/IE to tell them they should support opacity VS computed style correctly, seems they are returning the computed value currently.

Comment 29

[mozilla]

Changed my test case to use computed style and still see the exact same issue. Will upload new test case.

Comment 30

[mozilla]

Attachment: test case showing use of computed style VS style

This attachment shows the bug still triggers... you can change the COMPUTED variable to see what happens in either case, by default it is on and we see:

"style opacity change of 1 0 -> 1" right before it fades back in.

When not using computed styles the value jumps on subsequent fade outs too.

Tested here using 19.0.2.

Comment 31

[mozilla]

Confirmed bug exists in latest Aurora nightly on Linux using latest test case.

Please let me know if there's anything else you'd like changed in the case to make it more minimal. I removed the setTimeout which makes the visual glitch no longer noticeable but still displays for me what is the most important consequence: the jumping of the computed css attribute rather than the smooth transition.

Comment 32

[mozilla]

The visual glitch is also noticeable in both firefox 19 and the nightly for me in the latest test.

So summmary of problems (in both 19.0.2 and nightly):

1. Set opacity from 1 to 0: Computed/style opacity works as expected.
2. Set opacity from 0 to 1: Computed opacity jumps up but is transitioning 10ms later. Style opacity jumps up as expected.
3. Set opacity from '' to 1: Both computed and style are transitioning immediately and 10ms later. In this case I believe that immediately after the non-computed should be 1 rather than transitioning.

Comment 33

[Loic]

I'm still seeing the issue with the reporter's testcase, but not with yours, Nick.

Comment 34

[Loic]

(with the latest Nightly, off course)

Comment 35

[Nick Cameron [:nrc]]

[e-mail], Loic - thanks for the info!

I've been looking further into this, it is one of the most maddening bugs I've looked at for a while. I see the flash most times, but only if I have the logging turned on - no logging, no flash. Doing any logging from GDB has been only mildly successful because the IO and backtracing slows things down so much that I miss most of the transition. Any internal logging is showing exactly the expected values, so I am not much closer to actually finding out what is going on.

Investigations continue...

Comment 36

[mozilla]

Just confirmed the bug in Windows too. Nick, thanks for your hard work, I'm also doing nothing but fixing maddening bugs in my career on far more boring projects than firefox, so I'm still envious you get to work at mozilla :P

Comment 37

[Nick Cameron [:nrc]]

OK, so the fun thing is that it is not the logging that causes this bug, but the flush which happens when we check the opacity value (by calling getComputedValue). In fact it is only the first one in the test case that matters, that is the call before we set the opacity on the element to the final value.

What is happening (I think) is that at the end of the transition, the ElementPropertyTransition for opacity on the element is set to a sentinel value. Then we do the getComputedValue stuff which causes a flush. When we flush for this transition (EnsureStyleRuleFor), there is nothing to because we have the sentinel, but we still set mStyleRuleRefreshTime to the current refresh driver tick. Then we set the new opacity value and consider starting a new transition. This calls EnsureStyleRuleFor, but because we just set mStyleRuleRefreshTime, we don't actually do anything, and so we leave the non-animated opacity around without starting the transition. The next refresh driver tick we do the right thing, which is why we just see a one frame flash.

My fix is to not update mStyleRuleRefreshTime unless we actually update something, which fixes the problem. But I'm not really sure if this is the right fix. Possibly we should instead override the time stamp check if we know the value has been changed, but I don't know how that would work.

Comment 38

[Nick Cameron [:nrc]]

Attachment: patch

see the above comment for an explanation and why I'm not sure if this is the right fix.

Comment 39

[mozilla]

Hi Nick, just pointing out that the test also fails when accessing the property directly (.style.opacity) as well as when checking it through the computed value. You can check this by changing the "constant" in my latest JavaScript test case.

Comment 40

[mozilla]

Perhaps I have missed something though, maybe use of .style.opacity also triggers getComputedValue (whether incorrectly or due to an indirect calculation necessary)?

Comment 41

[Nick Cameron [:nrc]]

(In reply to mozilla from comment #39)
> Hi Nick, just pointing out that the test also fails when accessing the
> property directly (.style.opacity) as well as when checking it through the
> computed value. You can check this by changing the "constant" in my latest
> JavaScript test case.

I'm not exactly sure of the mechanism, but we end up tripping the same code both ways. My patch fixes the flash for me using the test case with .style.opacity path.

Comment 42

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 727503 [details] [diff] [review]
patch

So I'm pretty sure we don't want this patch, because it makes EnsureStyleRuleFor more expensive.  The current code maintains the invariant that EnsureStyleRuleFor does real work at most once per refresh cycle, and this breaks that.

I need to understand your explanation better to figure out what alternatives to suggest, though.

Comment 43

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

At a very high level I think the way this ought to work is that we should get an nsTransitionManager::StyleContextChanged call when there's a style change, and something (still need to figure out what) in StyleContextChanged should invalidate the style rule in the ElementTransitions.

Comment 44

[Nick Cameron [:nrc]]

Attachment: version 2

This seems to work too. I think this is what you meant be invalidating the style rule. I think that if we get this far in StyleContextChanged we have a style change and need to start a transition, and if we don't we don't need to invalidate. We still end up running EnsureStyleRule twice in one tick in that case, but as Ensure... gets called before StyleContextChanged, I don't think we can avoid that.

Comment 45

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 733193 [details] [diff] [review]
version 2

I'm inclined to say that you should move this *up*, to right before this code:

  if (!startedAny) {
    return nullptr;
  }

But there it would need to be:

  if (et) {
    et->mStyleRule = nullptr;
  }

since if !startedAny, et *might* be null.


I haven't completely thought through whether the difference matters, but I'm not sure it's worth it.  So r=dbaron with that

Comment 46

[Nick Cameron [:nrc]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc4066373f5a

Comment 47

[Ed Morley [:emorley]]

Backed out for failures in test_bug451286.xul:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=dc4066373f5a

https://hg.mozilla.org/integration/mozilla-inbound/rev/ca4c54c81073

Comment 48

[Nick Cameron [:nrc]]

We are failing content/chrome/toolkit/content/tests/chrome/test_bug451286.xul, where we apparently are not un-highlighting the find toolbar quickly enough. I cannot repro this outside the mochi test, but the mochi test consistently fails.

We are hitting the new code, i.e., setting mStyleRule to null, then accessing mStyleRule via WalkTransitionRule, but we are hitting EnsureStyleRule before we access mStyleRule, so it is not that we are looking before it is set. I'm a little bit stumped.

Comment 49

[Nick Cameron [:nrc]]

Dumping mStyleRule, we are indeed getting the new styles, rather than the old ones, which seems like a good thing?

Here are the new and old values, unfortunately I'd not sure which is the one which causes the test to fail:

old
NULL
new
[anim values] { opacity: 0; margin-bottom: -14.6667px; }

old
[anim values] { opacity: 0; margin-bottom: -14.6667px; }
new
[anim values] { visibility: visible; }

old
[anim values] { visibility: visible; }
new
[anim values] { opacity: 0; margin-bottom: -14.6667px; }

old
[anim values] { opacity: 0; margin-bottom: -14.6667px; }
new
[anim values] { visibility: visible; }

Where old is the value before we set it to nullptr (and the value without this patch) and new is the value after ensuring the style rule.

Comment 50

[Nick Cameron [:nrc]]

Actually, I just checked without this patch and then we get [anim values] { opacity: 0; margin-bottom: -14.6667px; } every time, i.e., we never see { visibility: visible; }. I do not know why.

Comment 51

[Nick Cameron [:nrc]]

dbaron: do you have any idea what is going on here? I can't decide if the test is buggy or we are doing something wrong in nsTranstionManager.

Comment 52

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Which test is failing?  And which snapshots are different?

If there are transitions involved, does the test need to wait for them to complete?  I don't see code in the test that does that.

Comment 53

[Nick Cameron [:nrc]]

(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #52)
> Which test is failing?  And which snapshots are different?
> 
content/chrome/toolkit/content/tests/chrome/test_bug451286.xul
noHighlightSnapshot and unhighlightSnapshot

> If there are transitions involved, does the test need to wait for them to
> complete?  I don't see code in the test that does that.

It might. I do not see code to wait either. I played around with this, but could not figure out how to add the waiting (most of my testing has been with reftests, mochi tests are still fairly unfamiliar territory).

How should I add a wait here?

Comment 54

[Nick Cameron [:nrc]]

Attachment: changes to test

Comment 55

[Nick Cameron [:nrc]]

So after a whole lot of debugging, it turns out that there wasn't a bug at all in the code (well maybe there is, but it doesn't affect the test, I'll investigate a little more and file another bug if necessary), but there is a bug in the test.

So, findBar.css defines |findbar| and |findbar[hidden]| styles. When opening and closing the findbar we transition opacity, margin-bottom, and visibility. The duration is 150ms for the first two and 0s for the third. The delay is set in |findbar[hidden]| and is 0 for the first two and 150ms for the third. That means the transition from collapse to visible when opening happens instantly, but the reverse transition when closing happens after 150ms. We therefore need to change the test to wait for this transition to finish before snapshotting.

Without the first patch the test (incorrectly) passes because at the time of the snapshot we still have the cached style rule on the ElementTransitions. The cached rule is from the start of the transition from closed to open and so still has |visibility: collapse|.

Comment 56

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 758350 [details] [diff] [review]
changes to test

I'm inclined to suggest that both part2 and part3 should begin with:

  if (aEvent.propertyName != "visibility") {
    return;
  }

(before the removeEventListener call), just to be a little more robust to somebody adjusting the transitions in the future (although it would still break if somebody added a transition on visibility for the opening direction or removed it from the closing direction.

(Though if that breaks things for some reason, just leave it the way it is.)

r=dbaron

Comment 57

[Nick Cameron [:nrc]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/711d46a1f67e
https://hg.mozilla.org/integration/mozilla-inbound/rev/a06a3cca82e8

Comment 58

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/711d46a1f67e
https://hg.mozilla.org/mozilla-central/rev/a06a3cca82e8

Comment 59

[Ryan VanderMeulen [:RyanVM]]

Backed out for causing frequent Android reftest-2 failures.
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f56fc0b80e3

https://tbpl.mozilla.org/php/getParsedLog.php?id=24064313&tree=Mozilla-Inbound

Comment 60

[Ed Morley [:emorley]]

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/6f56fc0b80e3

Comment 62

[Nick Cameron [:nrc]]

try push with more test changes: https://tbpl.mozilla.org/?tree=Try&rev=996781cb1604

Comment 63

[Nick Cameron [:nrc]]

Attachment: changes to another test

It looks like the focus is getting transitioned too on Android due to transition: all.

Comment 64

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 763465 [details] [diff] [review]
changes to another test

I think you could get away with only half of the changes (script or style, and I'd pick changing the style), though changing both is ok too.

(I feel like I've reviewed another patch to this test sometime in the past week.)

Comment 65

[Nick Cameron [:nrc]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ed83ea296382
https://hg.mozilla.org/integration/mozilla-inbound/rev/4bf28d2bed42
https://hg.mozilla.org/integration/mozilla-inbound/rev/1598fe77701f

Comment 66

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ed83ea296382
https://hg.mozilla.org/mozilla-central/rev/4bf28d2bed42
https://hg.mozilla.org/mozilla-central/rev/1598fe77701f