Closed Bug 1050877 Opened 6 years ago Closed 6 years ago

media.gmp-manager.url should mention GMP


(Core :: WebRTC: Audio/Video, defect)

Not set



Tracking Status
firefox33 --- fixed
firefox34 --- fixed


(Reporter: mccr8, Assigned: mastermind.aseem, Mentored)


(Whiteboard: [lang=js] [good first bug])


(1 file, 1 obsolete file)

Right now, it is set to dummy.xml, which made figuring out that bug 1046182 was GMP related much harder.  If the name was changed to something like dummy-gmp-manager-url.xml or something it would be more obvious what it is.
Component: Video/Audio → WebRTC: Audio/Video
georg: you want to take this?
Flags: needinfo?(georg.fritzsche)
Adding to our backlog - not taking it right now as it isn't critical and i have some things to finish up.
Points: --- → 2
Flags: needinfo?(georg.fritzsche) → firefox-backlog+
Mentor: georg.fritzsche
Points: 2 → 1
Whiteboard: [lang=js] [good first bug]
Attached patch bug1050877.patch (obsolete) — Splinter Review
I just gave this a try. Please review if I'm on the right track.
Attachment #8477859 - Flags: review?(georg.fritzsche)
Comment on attachment 8477859 [details] [diff] [review]

Review of attachment 8477859 [details] [diff] [review]:

Thank you, this looks good!
Looking at this now, i would change it to dummy-gmp-manager.xml (the "-url" part seems redundant as this is a URL) and then request review from jmaher for this.
Attachment #8477859 - Flags: review?(georg.fritzsche) → feedback+
Attached patch bug1050877.patchSplinter Review
Thank you for replying.
I dropped the -url and now it is simply "dummy-gmp-manager.xml"
Please review and confirm.
Attachment #8477859 - Attachment is obsolete: true
Attachment #8478536 - Flags: review?(jmaher)
Comment on attachment 8478536 [details] [diff] [review]

Review of attachment 8478536 [details] [diff] [review]:

Aseem, thanks for posting this patch.  In reading over this, I don't see where we defined dummy-gmp-manager.xml.  Do we expect it to really be accessible?  This patch does what the bug asks for, but I would like to learn more about why we are naming this.  Also should we be updating reftest and talos at all?
Attachment #8478536 - Flags: review?(jmaher) → review+
(In reply to Joel Maher (:jmaher) from comment #7)
> Aseem, thanks for posting this patch.  In reading over this, I don't see
> where we defined dummy-gmp-manager.xml.Do we expect it to really be
> accessible?

No, the previous URL wasn't accessible either. We just want things to fail sanely.

> Also should we be updating reftest and talos at all?

Talos was adressed here previously:
Where should reftest be adressed?
reftest is here:

I am not sure we need to do anything for reftest.  Glad you have taken care of these things!
Given this was just about naming the existing prefs, i think we're good here.
Trivial renaming, so i don't think we need a try run here.
Keywords: checkin-needed
Thanks for fixing this!
Assignee: nobody → mastermind.aseem
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Flags: qe-verify?
Flags: qe-verify? → qe-verify-
Iteration: --- → 34.3
You need to log in before you can comment on or make changes to this bug.