933275 - Regression: Active camera/microphone media icon has been replaced with a Firefox logo

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

Possible regression from bug 815202?

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2013-09-24&enddate=2013-09-25

We need to display the appropriate media in-use icons and not our browser logo.

Comment 1

[Gian-Carlo Pascutto [:gcp]]

Confirmed, also causes bug 932816.

Comment 2

[Wesley Johnston (:wesj)]

Attachment: Patch

Our api changed underneath us! We should move this whole thing to the new API. Maybe we'd be good to do that here, but I'm opting to fix this instead an we'll use a follow up for that :) (Also, I haven't quite landed the new API yet since the tree's been closed all week).

Comment 3

[Federico Paolinelli]

Comment on attachment 825735 [details] [diff] [review]
Patch

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

Looks good. Anyway, I saw the notifications api stuff landing on friday, so I think the WebrtcUI notifications completely broke in the meanwhile :-(

Comment 4

[Federico Paolinelli]

Moreover, I see the downloads button stuff has target milestone-> 27, whereas the notification api stuff has target milestone->28. Should we back it up until everything here is stable?

Comment 5

[Federico Paolinelli]

Attachment: bug-933275-fix

Here I ported webrtcui notifications to the new notifications api. I also added light parameter to notification options since it was not there before.

Comment 6

[Wesley Johnston (:wesj)]

Comment on attachment 827071 [details] [diff] [review]
bug-933275-fix

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

That wasn't too ugly

::: mobile/android/modules/Notifications.jsm
@@ +119,5 @@
>          };
>          msg.actions.push(obj);
>        }
>      }
> +    

WhiteSpace!

Comment 7

[Federico Paolinelli]

Attachment: bug-933275-fix

Just removed the ws

Comment 8

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/fx-team/rev/540035c526a7

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/540035c526a7

Comment 10

[Aaron Train [:aaronmt]]

We need this uplifted to mozilla-27 (Aurora).

Comment 11

[Wesley Johnston (:wesj)]

Comment on attachment 825735 [details] [diff] [review]
Patch

I don't think notifications.jsm is on Aurora. We'll need this patch there I guess.

Comment 12

[Wesley Johnston (:wesj)]

Comment on attachment 825735 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Notification buttons
User impact if declined: Wrong icon for webrtc
Testing completed (on m-c, etc.): Similar patch landed on central and all is ok.
Risk to taking this patch (and alternatives if risky): Low risk
String or IDL/UUID changes made by this patch: None.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5cd502cfa581