media.gmp-manager.url should mention GMP

RESOLVED FIXED in Firefox 33

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

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

Tracking

Trunk
mozilla34
Points:
1
Bug Flags:
firefox-backlog +
qe-verify -

Firefox Tracking Flags

(firefox33 fixed, firefox34 fixed)

Details

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

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

5 years ago
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.
Reporter

Updated

5 years ago
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]
Assignee

Comment 4

5 years ago
Posted 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]
bug1050877.patch

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+
Assignee

Comment 6

5 years ago
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]
bug1050877.patch

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:
https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1042161&attachment=8470802
Where should reftest be adressed?
reftest is here:
http://dxr.mozilla.org/mozilla-central/source/layout/tools/reftest/reftest-preferences.js

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
Reporter

Comment 11

5 years ago
Thanks for fixing this!
Assignee: nobody → mastermind.aseem
https://hg.mozilla.org/mozilla-central/rev/a019b75127fd
Status: NEW → RESOLVED
Closed: 5 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.