1011472 - Desktop client needs "incoming call" visual and audio (ringing) notification (alerting)

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect, P1)

Comment 1

[Romain Testard [:RT]]

Attachment: Incoming call audio notification (FFOS official)

Comment 2

[:shell escalante]

validate with Bill Maggs that we are going with the sound attached.  Darrin did the design - but we need an audio file.

Comment 3

[Bill Maggs (bmaggs)]

Mostly essential that sonically it is the same on FFOS phones and FF Desktop. I tried it on a Flame, Keon, and Desktop on a Mac and it is identical. It sounds a little sharp to my punk-rock destroyed ears but I assume the FFOS guys know what they're doing.

Comment 4

[Romain Testard [:RT]]

Incoming call visual notification when not signed in (link call-back only):
* URL name (custom or randomly generated)
* Information whether the caller's media selection is audio only or audio+video
* Link expiration date

Incoming call visual notification when signed in and receiving a link call-back:
* URL name (custom or randomly generated)
* Information whether the caller's media selection is audio only or audio+video
* Link expiration date

Incoming call visual notification when signed in and receiving a direct call:
* Caller's friendly name (or ID if no friendly name)
* Caller's avatar (or default avatar if not applicable)
* Information whether the caller's media selection is audio only or audio+video
* Link expiration date

Comment 5

[Adam Roach [:abr]]

Attachment: Add audio alert for incoming call

Comment 6

[Adam Roach [:abr]]

Attachment: Add audio alert for incoming call

Comment 7

[Adam Roach [:abr]]

This patch implements the needed functionality. I'm not sure what's available for testing this in an automated fashion, though.

Lacking a response on which tone to use, I'd propose landing this with the existing tone, and updating if necessary.

I'm going to poke around at this just a little more before I ask for review.

Comment 8

[Adam Roach [:abr]]

Attachment: Add audio alert for incoming call

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 8442389 [details] [diff] [review]
Add audio alert for incoming call

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

Some fly-by comments.

::: browser/components/loop/MozLoopAPI.jsm
@@ +22,5 @@
>   * See the documentation on the individual functions for details of the API.
>   *
>   * @param {nsIDOMWindow} targetWindow The content window to attach the API.
>   */
>  function injectLoopAPI(targetWindow) {

This part should be reviewed by a browser peer.

@@ +26,5 @@
>  function injectLoopAPI(targetWindow) {
> +  let chromeWindow = getChromeWindow(targetWindow);
> +  let ringer = new chromeWindow.Audio(Services.prefs.getCharPref("loop.ringtone"));
> +  ringer.loop = true;
> +  ringer.load();

Doing this will load the ringtone into the Panel as well as the Conversation window, that seems a bit of a waste.

Also, why not just load this via conversation.html like any other media resource?

::: browser/components/loop/content/js/conversation.js
@@ +54,5 @@
>       * @param  {MouseEvent} event
>       */
>      handleDecline: function(event) {
>        event.preventDefault();
> +      this.model.trigger("decline");

The current test infrastructure we have is Marionette, I'll pull together some docs on that today.

Comment 10

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Comment on attachment 8442389 [details] [diff] [review]
Add audio alert for incoming call

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

Some discussions to have around this :)

::: browser/components/loop/MozLoopAPI.jsm
@@ +26,5 @@
>  function injectLoopAPI(targetWindow) {
> +  let chromeWindow = getChromeWindow(targetWindow);
> +  let ringer = new chromeWindow.Audio(Services.prefs.getCharPref("loop.ringtone"));
> +  ringer.loop = true;
> +  ringer.load();

I second that. I'm not 100% sure but to me Audio loading and playing should be done in content (the mozLoop API would then only provide the ringtone file path to use), eg. from the conversation window. But maybe I'm missing a case where we'd want ringing happening with no conversation window opened?

If we want to go the content way, the Conversation model should trigger an event on incoming call so we could listen to it to start playing the sound, eg. from the Conversation router.

Another argument for the content approach is that maintaining this will be achievable by both content-only and platform people…

Last, tests would probably be easier to write if we move audio handling to content (imho) :)

@@ +182,5 @@
> +      enumerable: true,
> +      configurable: true,
> +      writable: true,
> +      value: function() {
> +        ringer.pause();

Shouldn't the sound be rewound?

@@ +210,5 @@
> +                   .getInterface(Ci.nsIWebNavigation)
> +                   .QueryInterface(Ci.nsIDocShellTreeItem)
> +                   .rootTreeItem
> +                   .QueryInterface(Ci.nsIInterfaceRequestor)
> +                   .getInterface(Ci.nsIDOMWindow);

As :standard8 said, I don't feel like reviewing this part as I'm not experienced enough with Gecko code & conventions.

::: browser/components/loop/content/js/conversation.js
@@ +54,5 @@
>       * @param  {MouseEvent} event
>       */
>      handleDecline: function(event) {
>        event.preventDefault();
> +      this.model.trigger("decline");

I don't get :standard8's comment about Marionette here…

Comment 11

[Mark Banner (:standard8)]

(In reply to Nicolas Perriault (:NiKo`) from comment #10)
> > +      this.model.trigger("decline");
> 
> I don't get :standard8's comment about Marionette here…

Adam was asking how we should test this, my comment was on a vaguely related area.

Comment 12

[Adam Roach [:abr]]

Comment on attachment 8442389 [details] [diff] [review]
Add audio alert for incoming call

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

I'll be uploading a refactored patch that delays loading the audio file until it is needed. Responses to other comments inine.

::: browser/components/loop/MozLoopAPI.jsm
@@ +22,5 @@
>   * See the documentation on the individual functions for details of the API.
>   *
>   * @param {nsIDOMWindow} targetWindow The content window to attach the API.
>   */
>  function injectLoopAPI(targetWindow) {

Thanks for the reminder -- I'll tag Mark Hammond on my next review.

@@ +26,5 @@
>  function injectLoopAPI(targetWindow) {
> +  let chromeWindow = getChromeWindow(targetWindow);
> +  let ringer = new chromeWindow.Audio(Services.prefs.getCharPref("loop.ringtone"));
> +  ringer.loop = true;
> +  ringer.load();

Yeah, I should have included some of my design rationale here. I expect that ringtone customization is going to be something that a relatively large set of users will be interested in, so having the sound file URL in a pref gives us the flexibility to do so; and, even if we don't get around to implementing it, putting the sound file in a pref gives add-on developers the chance to tweak this in an add-on.

That means we're using an absolute path -- a chrome URI (and, even if we weren't, any custom ringtone is probably going to be a file:// URL, which has the same permission properties). And you can't access a chrome URI from content permissions. There are, of course, different ways to factor this, but ultimately it means that we need to put this audio playout in chrome permissions, which puts it behind the mozLoop API.

The point about loading the sound into non-conversation panels is a good one; I'll move the loading down into the startAlerting call.

@@ +182,5 @@
> +      enumerable: true,
> +      configurable: true,
> +      writable: true,
> +      value: function() {
> +        ringer.pause();

Only if we plan to have alerting more than once per conversation. :)

I originally had a "ringer.currentTime = 0" in here, but it was generating some completely inscrutable error messages. Since it was kind of academic (given that you won't ring a call more than once), I didn't worry about it.

In any case, the refactoring to prevent loading audio into non-conversation panels will get the reset we want (I think).

@@ +210,5 @@
> +                   .getInterface(Ci.nsIWebNavigation)
> +                   .QueryInterface(Ci.nsIDocShellTreeItem)
> +                   .rootTreeItem
> +                   .QueryInterface(Ci.nsIInterfaceRequestor)
> +                   .getInterface(Ci.nsIDOMWindow);

I'm sure Mark Hammond can sign off on it, but this function is identical to the one that does the same thing in MozSocialAPI.jsm.

::: browser/components/loop/content/js/conversation.js
@@ +54,5 @@
>       * @param  {MouseEvent} event
>       */
>      handleDecline: function(event) {
>        event.preventDefault();
> +      this.model.trigger("decline");

I'm a little confused here, too. :)

Comment 13

[Adam Roach [:abr]]

Attachment: Add audio alert for incoming call

Comment 14

[Adam Roach [:abr]]

Comment on attachment 8442854 [details] [diff] [review]
Add audio alert for incoming call

I'm looking to Nico to review the changes to conversation.js and jar.mn; and to Florian to review the changes to firefox.js and to MozLoopAPI.jsm.

Comment 15

[Florian Quèze [:florian]]

(In reply to Adam Roach [:abr] from comment #12)

> putting the sound file in a pref gives add-on developers
> the chance to tweak this in an add-on.

Actually, putting the file in chrome:// is enough to give add-on authors an opportunity to override it.

> And you can't access a chrome URI from content permissions.

I think the contentaccessible=yes flag makes it possible since bug 292789 (and that flag is already there for chrome://browser/content http://mxr.mozilla.org/mozilla-central/source/browser/base/jar.mn#5). I haven't verified that you won't run into same-origin restrictions somewhere though.

Comment 16

[Florian Quèze [:florian]]

Comment on attachment 8442854 [details] [diff] [review]
Add audio alert for incoming call

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

Like others before, I would prefer the handling of the sound to be done completely in content, and you would only have a getAttention API to expose on mozLoop.

The following review comments assume that you do want to move forward with the current approach:

::: browser/app/profile/firefox.js
@@ +1511,5 @@
>  
>  #ifdef MOZ_LOOP
>  pref("loop.server", "https://loop.services.mozilla.com");
>  pref("loop.do_not_disturb", false);
> +pref("loop.ringtone", "chrome://browser/content/loop/shared/sounds/Firefox-Long.mp3");

Is mp3 the current preferred format for audio files included in the tree? (Asking mostly because I think I've seen .ogg files a few times, and I don't remember seeing an mp3 file.) Not sure if there's a policy somewhere about this.

::: browser/components/loop/MozLoopAPI.jsm
@@ +24,5 @@
>   * @param {nsIDOMWindow} targetWindow The content window to attach the API.
>   */
>  function injectLoopAPI(targetWindow) {
> +  let chromeWindow = getChromeWindow(targetWindow);
> +  let ringer = new chromeWindow.Audio();

Could this be done only when you are about the play a sound (which would only happen in conversation windows)?

@@ +161,5 @@
> +      enumerable: true,
> +      configurable: true,
> +      writable: true,
> +      value: function() {
> +        chromeWindow.getAttention();

Keeping the value of chromeWindow from when the mozLoop API was injected seems like it would cause trouble if the chat window is ever detached.

@@ +168,5 @@
> +        ringer.load();
> +        ringer.play();
> +        targetWindow.document.addEventListener("visibilitychange",
> +          function(event) {
> +            if(event.currentTarget.hidden) {

Nit: space between 'if' and '('.

@@ +183,5 @@
> +      enumerable: true,
> +      configurable: true,
> +      writable: true,
> +      value: function() {
> +        ringer.pause();

If you can create the audio element in startAlerting, setting ringer to null here should let it be GC'ed.

@@ +205,5 @@
>    // Handle window.close correctly on the panel and chatbox.
>    hookWindowCloseForPanelClose(targetWindow);
>  }
> +
> +function getChromeWindow(contentWin) {

I'm not sure this would play well with e10s if the chromeWindow and contentWindow aren't in the same process, but it seems all the urls where navigator.mozLoop is injected are about: urls, and I think these are always loaded in the chrome process.

Comment 17

[Adam Roach [:abr]]

(In reply to Florian Quèze [:florian] [:flo] from comment #15)
> (In reply to Adam Roach [:abr] from comment #12)
> 
> > putting the sound file in a pref gives add-on developers
> > the chance to tweak this in an add-on.
> 
> Actually, putting the file in chrome:// is enough to give add-on authors an
> opportunity to override it.
> 
> > And you can't access a chrome URI from content permissions.
> 
> I think the contentaccessible=yes flag makes it possible since bug 292789
> (and that flag is already there for chrome://browser/content
> http://mxr.mozilla.org/mozilla-central/source/browser/base/jar.mn#5). I
> haven't verified that you won't run into same-origin restrictions somewhere
> though.

Well, we run into some kind of permissions issue; if I try to run this with an Audio element associated with our content pages, I get errors of the form:

Security Error: Content at about:loopconversation#incoming/1403560194377 may not load or link to chrome://browser/content/loop/shared/sounds/Firefox-Long.mp3.

Comment 18

[Adam Roach [:abr]]

(In reply to Florian Quèze [:florian] [:flo] from comment #16)
> Comment on attachment 8442854 [details] [diff] [review]
> Add audio alert for incoming call
> 
> Review of attachment 8442854 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Like others before, I would prefer the handling of the sound to be done
> completely in content, and you would only have a getAttention API to expose
> on mozLoop.

Noted. However, I'm still at something of a loss as to how to play out chrome:// sounds from content-permissions about: URLs. I know that other attempts at rectifying origin issues in our current structure have been frustrated by some pretty serious complicating factors, and I don't want to block audio alerting on figuring those out.
 
> The following review comments assume that you do want to move forward with
> the current approach:
> 
> ::: browser/app/profile/firefox.js
> @@ +1511,5 @@
> >  
> >  #ifdef MOZ_LOOP
> >  pref("loop.server", "https://loop.services.mozilla.com");
> >  pref("loop.do_not_disturb", false);
> > +pref("loop.ringtone", "chrome://browser/content/loop/shared/sounds/Firefox-Long.mp3");
> 
> Is mp3 the current preferred format for audio files included in the tree?
> (Asking mostly because I think I've seen .ogg files a few times, and I don't
> remember seeing an mp3 file.) Not sure if there's a policy somewhere about
> this.

I've transcoded to ogg and verified that it still sounds reasonable. (I don't have golden ears, but using large external speakers here, I can't tell the difference between the formats).


> ::: browser/components/loop/MozLoopAPI.jsm
> @@ +24,5 @@
> >   * @param {nsIDOMWindow} targetWindow The content window to attach the API.
> >   */
> >  function injectLoopAPI(targetWindow) {
> > +  let chromeWindow = getChromeWindow(targetWindow);
> > +  let ringer = new chromeWindow.Audio();
> 
> Could this be done only when you are about the play a sound (which would
> only happen in conversation windows)?

Sure, that seems reasonable. Moved.

> @@ +161,5 @@
> > +      enumerable: true,
> > +      configurable: true,
> > +      writable: true,
> > +      value: function() {
> > +        chromeWindow.getAttention();
> 
> Keeping the value of chromeWindow from when the mozLoop API was injected
> seems like it would cause trouble if the chat window is ever detached.

I'm not sure how we end up with that kind of re-rooting prior to alerting, but the point is a good one in general. I've moved this call so we don't grab the chrome window until we're about to use it.

> @@ +168,5 @@
> > +        ringer.load();
> > +        ringer.play();
> > +        targetWindow.document.addEventListener("visibilitychange",
> > +          function(event) {
> > +            if(event.currentTarget.hidden) {
> 
> Nit: space between 'if' and '('.

Fixed.

> @@ +183,5 @@
> > +      enumerable: true,
> > +      configurable: true,
> > +      writable: true,
> > +      value: function() {
> > +        ringer.pause();
> 
> If you can create the audio element in startAlerting, setting ringer to null
> here should let it be GC'ed.

Done.

> @@ +205,5 @@
> >    // Handle window.close correctly on the panel and chatbox.
> >    hookWindowCloseForPanelClose(targetWindow);
> >  }
> > +
> > +function getChromeWindow(contentWin) {
> 
> I'm not sure this would play well with e10s if the chromeWindow and
> contentWindow aren't in the same process, but it seems all the urls where
> navigator.mozLoop is injected are about: urls, and I think these are always
> loaded in the chrome process.

If this is an issue for us, then it's an issue for Social: <http://dxr.mozilla.org/mozilla-central/source/toolkit/components/social/MozSocialAPI.jsm#269> Shane? Any thoughts here?

New patch forthcoming to address other review comments.

Comment 19

[Adam Roach [:abr]]

Attachment: Add audio alert for incoming call

Comment 20

[Florian Quèze [:florian]]

(In reply to Adam Roach [:abr] from comment #18)
> (In reply to Florian Quèze [:florian] [:flo] from comment #16)

> > Keeping the value of chromeWindow from when the mozLoop API was injected
> > seems like it would cause trouble if the chat window is ever detached.
> 
> I'm not sure how we end up with that kind of re-rooting prior to alerting

It may not be possible with the current Loop UI. I still had in mind the Talkilla UI where it was possible to start with text-chat-only and then add video to the call.

> > @@ +205,5 @@
> > >    // Handle window.close correctly on the panel and chatbox.
> > >    hookWindowCloseForPanelClose(targetWindow);
> > >  }
> > > +
> > > +function getChromeWindow(contentWin) {
> > 
> > I'm not sure this would play well with e10s if the chromeWindow and
> > contentWindow aren't in the same process, but it seems all the urls where
> > navigator.mozLoop is injected are about: urls, and I think these are always
> > loaded in the chrome process.
> 
> If this is an issue for us, then it's an issue for Social

I think Social is also currently running only in the parent process (except for the frameworker that is in the content process). I could be wrong on this, so I would also appreciated Shane's thoughts here :-).

Comment 21

[Florian Quèze [:florian]]

Comment on attachment 8444763 [details] [diff] [review]
Add audio alert for incoming call

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

::: browser/components/loop/MozLoopAPI.jsm
@@ +167,5 @@
> +        ringer.src = Services.prefs.getCharPref("loop.ringtone");
> +        ringer.loop = true;
> +        ringer.load();
> +        ringer.play();
> +        targetWindow.document.addEventListener("visibilitychange",

Should we remove this event listener when we are done playing the sound? I think it's fine to keep it, but I'm not fully awake anymore.

@@ +169,5 @@
> +        ringer.load();
> +        ringer.play();
> +        targetWindow.document.addEventListener("visibilitychange",
> +          function(event) {
> +            if (event.currentTarget.hidden) {

I think we now need to null check ringer here...

@@ +184,5 @@
> +    stopAlerting: {
> +      enumerable: true,
> +      configurable: true,
> +      writable: true,
> +      value: function() {

... and here.

Comment 22

[Florian Quèze [:florian]]

Comment on attachment 8444763 [details] [diff] [review]
Add audio alert for incoming call

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

r=me for the firefox.js and MozLoopAPI.jsm changes, with the 2 null checks requested in comment 21 added.

Comment 23

[Adam Roach [:abr]]

Attachment: Add audio alert for incoming call

Comment 24

[Adam Roach [:abr]]

Comment on attachment 8445406 [details] [diff] [review]
Add audio alert for incoming call

This patch incorporates Florian's comments (including removal of the event handler when it is no longer needed). Now I'm looking for review on the changes to conversation.js and jar.mn.

Comment 25

[Shane Caraveo (:mixedpuppy)]

> > > > +
> > > > +function getChromeWindow(contentWin) {
> > > 
> > > I'm not sure this would play well with e10s if the chromeWindow and
> > > contentWindow aren't in the same process, but it seems all the urls where
> > > navigator.mozLoop is injected are about: urls, and I think these are always
> > > loaded in the chrome process.
> > 
> > If this is an issue for us, then it's an issue for Social
> 
> I think Social is also currently running only in the parent process (except
> for the frameworker that is in the content process). I could be wrong on
> this, so I would also appreciated Shane's thoughts here :-).

Without trying, I'd be fairly certain it will break if e10s is turned on.  There are a couple places in socialapi that will likely break with e10s, frameworker is the only thing generally tested for e10s.

A quick mxr search shows several places are using this slightly different mechanism, but I'm not sure if that is e10s safe either:

https://mxr.mozilla.org/mozilla-central/source/toolkit/components/social/SocialService.jsm#549

I would suggest getting someone on #fx-team who knows the e10s stuff better than me for a good answer.

Comment 26

[Maire Reavy [:mreavy]]

Hi Marco -- Why did this bug get pulled for the Desktop 33.2 sprint?  Is Darrin doing more UX work for "incoming call" notification?  If so, can we open a new bug?  Adam (Abr) has a patch for the initial implementation -- up for review now.  So I think it makes sense to finish that off here (in this bug). 

Also, I think there may be a few other Loop::Client bugs that got pulled for UX work this Sprint that are actually Loop:: Client implementation bugs.  I'm happy to provide a list of the bugs that I think got mis-pulled.  Thanks!

Comment 27

[Adam Roach [:abr]]

I've opened Bug 1030253 to capture any further alerting-related UX work. This patch needs to land approximately last week, so I'm going to unhook it from future UX refinements. We can, of course, revector the experience once designs are in place, but we need something functional Right Now.

Comment 28

[Marco Mucci [:MarcoM]]

(In reply to Maire Reavy [:mreavy] (Plz needinfo me, PTO on Jun 23) from comment #26)
> Hi Marco -- Why did this bug get pulled for the Desktop 33.2 sprint?  Is
> Darrin doing more UX work for "incoming call" notification?  If so, can we
> open a new bug?  Adam (Abr) has a patch for the initial implementation -- up
> for review now.  So I think it makes sense to finish that off here (in this
> bug). 
> 
> Also, I think there may be a few other Loop::Client bugs that got pulled for
> UX work this Sprint that are actually Loop:: Client implementation bugs. 
> I'm happy to provide a list of the bugs that I think got mis-pulled.  Thanks!

Hi Maire,  I didn't pull the bug out of the iteration.  Please see Comment #27 - Adam removed out iteration tags.  Should it remain?

Comment 29

[Marco Mucci [:MarcoM]]

Adding the new Bug 1030253 - [UX] Refined UX: Desktop client needs "incoming call" visual and audio (ringing) notification (alerting) to Iteration 33.2

Comment 30

[Mark Banner (:standard8)]

Comment on attachment 8445406 [details] [diff] [review]
Add audio alert for incoming call

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

General note, that the ringing stops if I expand the chat window out to a separate window before accepting the call. Not a big issue, so I think we can fix that later.

The patch looks good, but currently the unit tests fail, so we need to get those fixed up and extended for these changes.

::: browser/components/loop/content/js/conversation.js
@@ +54,5 @@
>       * @param  {MouseEvent} event
>       */
>      handleDecline: function(event) {
>        event.preventDefault();
> +      this.model.trigger("decline");

The marionette tests currently fail for these changes. If you need help updating them, I can help you.

You can run them either via:

./mach marionette-test browser/components/loop/manifest.ini

or manually by going into browser/components/loop/standalone and running

$ make install
$ make runserver

and then visiting http://localhost:3000.

::: browser/components/loop/jar.mn
@@ +10,5 @@
>    content/browser/loop/shared/css/conversation.css   (content/shared/css/conversation.css)
>    content/browser/loop/shared/img/icon_32.png        (content/shared/img/icon_32.png)
>    content/browser/loop/shared/img/icon_64.png        (content/shared/img/icon_64.png)
>    content/browser/loop/shared/img/loading-icon.gif   (content/shared/img/loading-icon.gif)
> +  content/browser/loop/shared/sounds/Firefox-Long.ogg   (content/shared/sounds/Firefox-Long.ogg)

nit: Can you put this under libs, to keep in approx alphabetical order?

Comment 31

[Adam Roach [:abr]]

Attachment: Add audio alert for incoming call

Comment 32

[Adam Roach [:abr]]

Comment on attachment 8446744 [details] [diff] [review]
Add audio alert for incoming call

Tests now pass, nit addressed.

Comment 33

[Mark Banner (:standard8)]

Comment on attachment 8446744 [details] [diff] [review]
Add audio alert for incoming call

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

Ok, looks better r=Standard8 with the other changes to the tests mentioned below.

::: browser/components/loop/test/desktop-local/conversation_test.js
@@ +146,5 @@
>          });
> +
> +        it("should stop alerting", function() {
> +          sandbox.stub(window.navigator.mozLoop, "stopAlerting");
> +          router.decline();

This should be router.accept()

@@ +184,5 @@
>        });
>  
> +      describe("#decline", function() {
> +        it("should close the window", function() {
> +          sandbox.stub(window, "close");

This stub needs to be moved to a beforeEach, otherwise (especially when testing locally), the window.close can be activated in the following test.

Comment 34

[Adam Roach [:abr]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/40139a7bd1b1

Comment 35

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/40139a7bd1b1

Comment 36

[u279076]

Does this bug need manual testing or is it sufficiently covered by automation?

Comment 37

[u279076]

Marking this verified fixed based on recent smoketesting.