Closed Bug 933275 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox for Android :: General, defect)

28 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 28
Tracking Status
firefox26 --- unaffected
firefox27 + fixed
firefox28 --- verified
fennec 27+ ---

People

(Reporter: aaronmt, Assigned: wesj)

References

Details

(Keywords: regression, reproducible)

Attachments

(2 files, 1 obsolete file)

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.
Version: Firefox 26 → Firefox 28
Confirmed, also causes bug 932816.
Assignee: nobody → wjohnston
tracking-fennec: ? → 27+
Attached patch PatchSplinter Review
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).
Attachment #825735 - Flags: review?(fedepaol)
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 :-(
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?
Attached patch bug-933275-fix (obsolete) — Splinter Review
Here I ported webrtcui notifications to the new notifications api. I also added light parameter to notification options since it was not there before.
Attachment #825735 - Attachment is obsolete: true
Attachment #825735 - Flags: review?(fedepaol)
Attachment #827071 - Flags: review?(wjohnston)
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!
Attachment #827071 - Flags: review?(wjohnston) → review+
Attached patch bug-933275-fixSplinter Review
Just removed the ws
Attachment #827071 - Attachment is obsolete: true
Attachment #827611 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/540035c526a7
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
We need this uplifted to mozilla-27 (Aurora).
Status: RESOLVED → VERIFIED
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.
Attachment #825735 - Attachment is obsolete: false
Attachment #825735 - Flags: review?(fedepaol)
Attachment #825735 - Flags: review?(fedepaol) → review+
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.
Attachment #825735 - Flags: approval-mozilla-aurora?
Attachment #825735 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.