Closed
Bug 782289
Opened 10 years ago
Closed 10 years ago
Headphones status notification
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: gerard-majax, Assigned: gerard-majax)
Details
Attachments
(2 files, 2 obsolete files)
2.23 KB,
patch
|
Details | Diff | Splinter Review | |
1.07 KB,
patch
|
Details | Diff | Splinter Review |
We should be able to forward the notification of headphones plugging/unplugging from Gonk to Chrome, so for example we can add an icon in statusbar when the headphones are plugged in.
Assignee | ||
Comment 1•10 years ago
|
||
A first patch that fires an event which we will later catch to create a mozChromeEvent.
Assignee | ||
Comment 2•10 years ago
|
||
Now the mozChromeEvent creation that will be later catched by statusbar.
Updated•10 years ago
|
Assignee: nobody → lissyx+mozillians
Updated•10 years ago
|
Attachment #651398 -
Flags: review?(21)
Updated•10 years ago
|
Attachment #651399 -
Flags: review?(21)
Comment 3•10 years ago
|
||
Comment on attachment 651398 [details] [diff] [review] Generating event from Gonk Redirecting the review since I'm not qualified to r? cpp changes in dom/system/gonk/AudioManager.cpp
Attachment #651398 -
Flags: review?(21) → review?(philipp)
Attachment #651399 -
Flags: review?(21) → review+
Comment 4•10 years ago
|
||
Comment on attachment 651398 [details] [diff] [review] Generating event from Gonk Review of attachment 651398 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/AudioManager.cpp @@ +60,5 @@ > +{ > + nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService(); > + if (obs) { > + if (aState == SWITCH_STATE_ON) { > + obs->NotifyObservers(nullptr, "headphones-status", NS_LITERAL_STRING("on").get()); Better topic name: "headphones-status-changed". Also, please const all strings (the topic and the three different data strings). #defines at the top of the file are ok. r=me with that.
Attachment #651398 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #651398 -
Attachment is obsolete: true
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #651399 -
Attachment is obsolete: true
Comment 7•10 years ago
|
||
Why does the r+'d review got obsolete? Did the patch got updated? I believe the next step should be uploading "Patch for commit" patches with commit messages that confirms the format m-c commits and use the "checkin-needed" keyword.
Comment 8•10 years ago
|
||
Hey Tim, Yeah, if you upload a new patch, then it removes the r+. If the r+ just had some nits to address and that's all you did in the new patch, then I normally put the patch as r+ and myself, but when I push the patch I put r=reviewer. I think that's the correct flow.
Assignee | ||
Comment 9•10 years ago
|
||
ping?
Comment 10•10 years ago
|
||
(In reply to Alexandre LISSY from comment #9) > ping? Oops, what happened here? Did we forget to check these patches in? (Alexandre, when you've uploaded final patches for a bug, it's a good idea to set the "checkin-needed" keyword on the bug.) Anyway, are these patches are good to go? They still apply on m-c.
Comment 11•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/67441523f021 https://hg.mozilla.org/integration/mozilla-inbound/rev/f608672b8fdf
Comment 12•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/67441523f021 https://hg.mozilla.org/mozilla-central/rev/f608672b8fdf
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•10 years ago
|
||
I just noticed that it's not the good patches that have been merged ? Looking at https://hg.mozilla.org/mozilla-central/rev/67441523f021 I don't see the changes I did after suggestions of comment 4.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 14•10 years ago
|
||
I think I fixd it up. https://hg.mozilla.org/integration/mozilla-inbound/rev/76ef86e0fd8c
Comment 15•10 years ago
|
||
And the second part here: https://hg.mozilla.org/integration/mozilla-inbound/rev/a8f996f5815d
Comment 16•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/76ef86e0fd8c https://hg.mozilla.org/mozilla-central/rev/a8f996f5815d
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•