Closed
Bug 1050877
Opened 10 years ago
Closed 10 years ago
media.gmp-manager.url should mention GMP
Categories
(Core :: WebRTC: Audio/Video, defect)
Core
WebRTC: Audio/Video
Tracking
()
People
(Reporter: mccr8, Assigned: mastermind.aseem, Mentored)
Details
(Whiteboard: [lang=js] [good first bug])
Attachments
(1 file, 1 obsolete file)
2.15 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
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•10 years ago
|
Component: Video/Audio → WebRTC: Audio/Video
Comment 2•10 years ago
|
||
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+
Updated•10 years ago
|
Mentor: georg.fritzsche
Points: 2 → 1
Whiteboard: [lang=js] [good first bug]
Comment 3•10 years ago
|
||
There's two places where the dummy.xml URL has to be changed: http://mxr.mozilla.org/mozilla-central/search?string=%2Fdummy.xml&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
Assignee | ||
Comment 4•10 years ago
|
||
I just gave this a try. Please review if I'm on the right track.
Attachment #8477859 -
Flags: review?(georg.fritzsche)
Comment 5•10 years ago
|
||
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•10 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 7•10 years ago
|
||
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+
Comment 8•10 years ago
|
||
(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?
Comment 9•10 years ago
|
||
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!
Comment 10•10 years ago
|
||
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
Comment 12•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a019b75127fd
Keywords: checkin-needed
Comment 13•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/c6a1dec30988
status-firefox33:
--- → fixed
status-firefox34:
--- → fixed
Comment 14•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a019b75127fd
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Updated•10 years ago
|
Flags: qe-verify?
Updated•10 years ago
|
Flags: qe-verify? → qe-verify-
Updated•10 years ago
|
Iteration: --- → 34.3
You need to log in
before you can comment on or make changes to this bug.
Description
•