Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Voicemail features #2593

Merged
merged 6 commits into from

4 participants

@marshall
Owner

This is the initial UI work for the platform features in these bugs:
https://bugzilla.mozilla.org/show_bug.cgi?id=736710
https://bugzilla.mozilla.org/show_bug.cgi?id=773068

(Note that this depends on those landing before it can be merged)

@vingtetun
Collaborator

/botio lint

@ghost

From: Bot.io (Main)


Received

Command cmd_lint from @vingtetun received. Current queue size: 0

Live output at: http://50.116.11.35:8000/f2a779056105ca8/output.txt

@ghost

From: Bot.io (Main)


Failed

Full output at http://50.116.11.35:8000/f2a779056105ca8/output.txt

Total script time: 1.08 mins

Lint: FAILED
----Skipping 5 file(s).
----- /apps/system/js/statusbar.js
307, E:0131: Single-quoted string preferred over double-quoted string.
311, E:0131: Single-quoted string preferred over double-quoted string.
318, E:0131: Single-quoted string preferred over double-quoted string.
326, E:0131: Single-quoted string preferred over double-quoted string.
344, E:0131: Single-quoted string preferred over double-quoted string.
344, E:0110: Line too long (84 characters).
345, E:0131: Single-quoted string preferred over double-quoted string.
Found 7 errors, including 0 new errors, in 1 files (190 files OK).

Some of the errors reported by GJsLint may be auto-fixable using the script
fixjsstyle. Please double check any changes it makes and report any bugs. The
script can be run by executing:

fixjsstyle --nojsdoc -r apps -e cubevid,crystalskull,towerjelly,email/js/ext,music/js/ext,calendar/js/ext
make: *** [lint] Error 1

@marshall
Owner

lint errors cleaned up and rebased

apps/dialer/index.html
@@ -56,7 +56,7 @@
<div class="keypad-key-label-container">
<div class="keypad-key-label" data-value="1">
<span class="keypad-number font-light">1</span>
- <span class="keypad-text font-semibold"></span>
+ <img class="keypad-subicon" src="style/images/voicemail.png"/>
@etiennesegonzac Collaborator

For consistency purpose I'd set an image background in css like the other ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
apps/dialer/index.html
@@ -80,7 +80,7 @@
<div class="keypad-key-label-container">
<div class="keypad-key-label keypad-key-label-centered" data-value="*">
<span>
- <div class="asterisk"></div>
+ <div class="keypad-icon asterisk"></div>
@etiennesegonzac Collaborator

why the change?

@marshall Owner

I had originally tried to borrow some CSS properties from keypad-icon for keypad-subicon, but it didn't work out. I can remove this...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
apps/dialer/js/keypad.js
@@ -304,7 +304,7 @@ var KeypadManager = {
// Sending the DTMF tone
var telephony = navigator.mozTelephony;
- if (telephony) {
+ if (telephony && telephony.startTone) {
@etiennesegonzac Collaborator

Didn't knew we could have one without he other. good to know!

@marshall Owner

I ran into this on the desktop build, I'm not sure why startTone didn't exist, but it helped me with testing so :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
apps/dialer/js/keypad.js
@@ -325,6 +325,14 @@ var KeypadManager = {
self._updatePhoneNumberView();
}, 400, this);
}
+
+ // Voicemail long press (needs to be longer since it actually dials)
+ if (key == '1') {
+ this._holdTimer = setTimeout(function(self) {
@etiennesegonzac Collaborator

This function deserves a name :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
tests/browser_dialer_contacts_edition.js
@@ -50,6 +50,8 @@ function testContactsEdition(window, document, nextStep) {
nextStep();
@etiennesegonzac Collaborator

Wow, did you run those tests? :)

Sadly we aren't able to move them to the new testing tools right now (explaining why they're still there).
But I don't think they run anymore :/

@marshall Owner

Hrmm.. I should have but I didn't :) I added them for completeness since I tweaked the Contact callback behavior..

@etiennesegonzac Collaborator

Then I'd vote for not touching the file, so we can see it's old and we should pay our test-debt :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@etiennesegonzac etiennesegonzac commented on the diff
build/preferences.js
@@ -90,6 +90,10 @@ let permissions = {
"urls": [],
"pref": "dom.mozBluetooth.whitelist"
},
+ "voicemail": {
+ "urls": [],
+ "pref": "dom.voicemail.whitelist"
@etiennesegonzac Collaborator

Out of curiosity, is it the same whitelist mechanism than WebTelephony (ie full URLs) or is it just on an origin base?

@marshall Owner

In my implementation I ensured it was origin based for simplicity (ala MobileNetwork etc)

@etiennesegonzac Collaborator

Origin based is good! Thanks for the info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@etiennesegonzac etiennesegonzac commented on the diff
apps/system/locales/system.en-US.properties
@@ -9,6 +9,11 @@ unreadMessages={[ plural(n) ]}
unreadMessages[zero] = You have no unread messages
unreadMessages[one] = You have one unread message
unreadMessages[other] = You have {{n}} unread messages
+newVoicemails={[ plural(n) ]}
+newVoicemails[one] = You have one new voicemail message
+newVoicemails[other] = You have {{n}} new voicemail messages
+newVoicemailsUnknown = You have new voicemail message(s)
+dialNumber = Dial {{number}}
missedCalls={[ plural(n) ]}
@etiennesegonzac Collaborator

Kudos on adding the new strings!

@marshall Owner

Will we need a translator to add these the other locales as well?

@etiennesegonzac Collaborator

The answer for this type of questions is to ping @fabi1cazenave :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
apps/system/js/statusbar.js
((39 lines not shown))
+ if (voicemailNumber) {
+ text = _('dialNumber', { number: voicemailNumber });
+ }
+
+ this.hideVoicemailNotification();
+
+ if (status.hasMessages) {
+ window.navigator.mozApps.getSelf().onsuccess = (function(event) {
+ var app = event.target.result;
+ var icon = app.installOrigin + '/style/statusbar/images/voicemail.png';
+ this.showVoicemailNotification(title, text, icon, voicemailNumber);
+ }).bind(this);
+ }
+ },
+
+ showVoicemailNotification: function sb_showVoicemailNotification(title, text,
@etiennesegonzac Collaborator

I really think you can get away by using a regular mozNotification here and get a ton a functionality for free :)
I you have callback issues, the bug if known and you can use https://github.com/mozilla-b2g/gaia/blob/master/apps/dialer/js/notification_helper.js

@marshall Owner

The current notification API doesn't allow me to remove the notification when the voicemail status changes.. is there a bug for this already?

@etiennesegonzac Collaborator

That's why! We talked about it that's right :)

The icon in the status bar should disappear but I see no harm is leaving an entry in the notification tray... what do you think?

@marshall Owner

I think it would be confusing to have a notification that says "You have new voicemails" after you've already listened to them :)

@etiennesegonzac Collaborator

You're right, I'll go over this part of the code again.

@etiennesegonzac Collaborator

Okay, I don't see how you can reuse more of the notification code, this is fine.

But can you extract all the fake notification code to a system/js/voicemail.js and keep only the status bar icon management here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@etiennesegonzac
Collaborator

You might experience some pain in rebasing this against the master with the new statusbar.
You can ping @timdream or myself for help.

marshall added some commits
@marshall marshall Addressed Voicemail review comments f3edaaf
@marshall marshall Merge in newest gaia master, update voicemail in the new statusbar UI
Conflicts:
	apps/system/index.html
	apps/system/js/notifications.js
	apps/system/js/statusbar.js
	apps/system/style/statusbar/statusbar.css
3523519
@etiennesegonzac
Collaborator

Can you do the system/js/voicemail.jsextraction?

@marshall
Owner

Yup, will do

@etiennesegonzac
Collaborator

Thanks!

@etiennesegonzac
Collaborator

Looks good!

@timdream the status bar icon is not part of icons.png, is it a blocker to merge this?

@vingtetun
Collaborator

@marshall The pull request is bitrotted, can you update it?

@etiennesegonzac
Collaborator

@timdream re-ping

@etiennesegonzac
Collaborator

Merging, using the sprited icon can be done in a follow up.

@etiennesegonzac etiennesegonzac merged commit 1217117 into mozilla-b2g:master
@timdream
Owner

@etiennesegonzac Sorry, I didn't see this up until now.

@etiennesegonzac
Collaborator

@timdream np, github's notifications sometime suck :)

@timdream
Owner

@etiennesegonzac We should sit in the same room. Really!

It doesn't make sense to me (nor to @jcarpenter) on adding another icon for the voice mail, since the notification icon on the left will be updated. So I am going to remove it. Sorry :-/

@etiennesegonzac You do want to make sure this kind of notification from system app (instead of mozChromeEvent) does get displayed on both lock screen and utility tray though.

@etiennesegonzac
Collaborator

@timdream till we get enough room for everybody in one place we'll still need to move and grab people :)

Next time I'll come find you.

I have absolutely no issue with the removal of this icon.
The spec probably didn't made it through @marshall

@marshall
Owner

@etiennesegonzac @timdream I asked for the spec a few weeks ago but no one seemed to know anything about voicemail :) I just went ahead, and figured you guys would know what to do once you have something to look at :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 19, 2012
  1. @marshall

    Initial gaia work for Voicemail notifications and dialing.

    marshall authored
    See also Bugs 736710 and 773068
Commits on Jul 20, 2012
  1. @marshall
  2. @marshall

    Merge in newest gaia master, update voicemail in the new statusbar UI

    marshall authored
    Conflicts:
    	apps/system/index.html
    	apps/system/js/notifications.js
    	apps/system/js/statusbar.js
    	apps/system/style/statusbar/statusbar.css
Commits on Jul 23, 2012
  1. @marshall
  2. @marshall
Commits on Jul 24, 2012
  1. @marshall

    Merge branch 'master' of git://github.com/mozilla-b2g/gaia into voice…

    marshall authored
    …mail
    
    Conflicts:
    	apps/dialer/js/keypad.js
This page is out of date. Refresh to see the latest.
View
2  apps/dialer/index.html
@@ -45,7 +45,7 @@
<div class="keypad-key-label-container">
<div class="keypad-key-label" data-value="1">
<span class="keypad-number font-light">1</span>
- <span class="keypad-text font-semibold"></span>
+ <div class="keypad-subicon voicemail"></div>
</div>
</div>
</div>
View
7 apps/dialer/js/contacts.js
@@ -17,12 +17,17 @@ var Contacts = {
if (mozContacts) {
var request = mozContacts.find(options, callback);
request.onsuccess = function findCallback() {
- if (request.result.length == 0)
+ if (request.result.length == 0) {
+ callback(null);
return;
+ }
var contacts = request.result;
callback(contacts[0]);
};
+ request.onerror = function findError() {
+ callback(null, request.error);
+ };
} else {
callback(null);
}
View
15 apps/dialer/js/keypad.js
@@ -334,6 +334,14 @@ var KeypadManager = {
self._updatePhoneNumberView();
}, 400, this);
}
+
+ // Voicemail long press (needs to be longer since it actually dials)
+ if (key == '1') {
+ this._holdTimer = setTimeout(function vm_call(self) {
+ self._longPress = true;
+ self._callVoicemail();
+ }, 3000, this);
+ }
} else if (event.type == 'mouseup') {
// If it was a long press our work is already done
if (this._longPress) {
@@ -380,5 +388,12 @@ var KeypadManager = {
}
this._holdTimer = null;
+ },
+
+ _callVoicemail: function kh_callVoicemail() {
+ var voicemail = navigator.mozVoicemail;
+ if (voicemail && voicemail.number) {
+ CallHandler.call(voicemail.number);
+ }
}
};
View
19 apps/dialer/js/oncall.js
@@ -218,9 +218,7 @@ var OnCallHandler = {
setupForCall: function och_setupForCall(call, typeOfCall) {
this.currentCall = call;
- this.lookupContact(call.number);
-
- CallScreen.update(call.number);
+ this.updateCallNumber(call.number);
CallScreen.render(typeOfCall);
@@ -348,8 +346,21 @@ var OnCallHandler = {
!navigator.mozTelephony.speakerEnabled;
},
- lookupContact: function och_lookupContact(number) {
+ updateCallNumber: function och_updateCallNumber(number) {
+ var voicemail = navigator.mozVoicemail;
+ if (voicemail) {
+ if (voicemail.number == number) {
+ CallScreen.update(voicemail.displayName);
+ return;
+ }
+ }
+
Contacts.findByNumber(number, function lookupContact(contact) {
+ if (!contact) {
+ CallScreen.update(number);
+ return;
+ }
+
if (contact.name) {
CallScreen.update(contact.name + ' - ');
}
View
1  apps/dialer/manifest.webapp
@@ -8,6 +8,7 @@
},
"permissions": [
"telephony",
+ "voicemail",
"contacts",
"power",
"mozApps",
View
BIN  apps/dialer/style/images/voicemail.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
View
19 apps/dialer/style/keypad.css
@@ -114,6 +114,25 @@
font-size: 2rem;
}
+.keypad-subicon {
+ background-repeat: no-repeat;
+ background-position: center bottom;
+
+ -moz-user-select: none;
+ pointer-events: none;
+
+ position: absolute;
+ left: 3.75rem;
+ bottom: 0.75rem;
+
+ width: 3rem;
+ height: 3rem;
+}
+
+.voicemail {
+ background-image: url('images/voicemail.png');
+}
+
.asterisk {
background-image: url('images/asterisk.png');
background-repeat: no-repeat;
View
5 apps/system/index.html
@@ -96,6 +96,9 @@
<!-- Wifi -->
<script defer src="js/wifi.js"></script>
+ <!-- Voicemail -->
+ <script defer src="js/voicemail.js"></script>
+
<!-- Theme and localization -->
<link rel="stylesheet" type="text/css" href="style/themes/default/system.css">
<link rel="resource" type="application/l10n" href="locales/locales.ini">
@@ -258,7 +261,6 @@ <h2 id="lockscreen-emergency-title" data-l10n-id="emergency-call">Emergency Call
<div id="statusbar">
<!-- Loading -->
<div id="statusbar-loading" hidden></div>
-
<!-- Notification -->
<div id="statusbar-notification" class="sb-start sb-icon sb-icon-notification"
data-num="" hidden></div>
@@ -285,6 +287,7 @@ <h2 id="lockscreen-emergency-title" data-l10n-id="emergency-call">Emergency Call
<div id="statusbar-mute" class="sb-icon sb-icon-mute" hidden></div>
<div id="statusbar-recording" class="sb-icon sb-icon-recording" hidden></div>
<div id="statusbar-sms" class="sb-icon sb-icon-sms" hidden></div>
+ <div id="statusbar-voicemail" data-show-num="false" data-num="" hidden></div>
<div id="statusbar-geolocation" class="sb-icon sb-icon-geolocation" hidden></div>
<div id="statusbar-usb" class="sb-icon sb-icon-usb" hidden></div>
</div>
View
1  apps/system/js/notifications.js
@@ -232,6 +232,7 @@ var NotificationScreen = {
}).bind(this), this.TOASTER_TIMEOUT);
this.updateStatusBarIcon(true);
+ return notificationNode;
},
removeNotification: function ns_removeNotification(notificationID) {
View
42 apps/system/js/statusbar.js
@@ -19,7 +19,7 @@ var StatusBar = {
this.getAllElements();
var settings = {
- 'ril.radio.disabled': ['signal', 'data'],
+ 'ril.radio.disabled': ['signal', 'data', 'voicemail'],
'ril.data.enabled': ['data'],
'wifi.enabled': ['wifi'],
'bluetooth.enabled': ['bluetooth'],
@@ -72,6 +72,9 @@ var StatusBar = {
this.update.data.call(this);
break;
+ case 'statuschanged':
+ this.update.voicemail.call(this);
+ break;
}
},
@@ -107,9 +110,15 @@ var StatusBar = {
if (bluetooth) {
// XXX need a reliable way to see if bluetooth is currently
// connected or not here.
-
this.update.bluetooth.call(this);
}
+
+ var voicemail = window.navigator.mozVoicemail;
+ if (voicemail) {
+ voicemail.addEventListener('statuschanged', this);
+ this.update.voicemail.call(this);
+ }
+
} else {
clearTimeout(this._clockTimer);
@@ -125,6 +134,11 @@ var StatusBar = {
conn.removeEventListener('voicechange', this);
conn.removeEventListener('datachange', this);
}
+
+ var voicemail = window.navigator.mozVoicemail;
+ if (voicemail) {
+ voicemail.removeEventListener('statuschanged', this);
+ }
}
},
@@ -335,6 +349,28 @@ var StatusBar = {
// this.icon.sms.dataset.num = ?;
},
+ voicemail: function sb_updateVoicemail() {
+ var voicemail = window.navigator.mozVoicemail;
+ if (!voicemail) {
+ return;
+ }
+
+ var status = voicemail.status;
+ if (!status) {
+ return;
+ }
+
+ var showCount = status.hasMessages && status.messageCount > 0;
+ this.icons.voicemail.hidden = !status.hasMessages;
+ this.icons.voicemail.dataset.showNum = showCount;
+
+ if (showCount) {
+ this.icons.voicemail.dataset.num = status.messageCount;
+ }
+
+ Voicemail.updateNotification(status);
+ },
+
geolocation: function sb_updateGeolocation() {
// XXX no way to probe active state of Geolocation
// this.icon.geolocation.hidden = ?
@@ -367,7 +403,7 @@ var StatusBar = {
var elements = ['notification', 'time',
'battery', 'wifi', 'data', 'flight-mode', 'conn', 'signal',
'tethering', 'alarm', 'bluetooth', 'mute',
- 'recording', 'sms', 'geolocation', 'usb'];
+ 'recording', 'sms', 'voicemail', 'geolocation', 'usb'];
var toCamelCase = function toCamelCase(str) {
return str.replace(/\-(.)/g, function replacer(str, p1) {
View
83 apps/system/js/voicemail.js
@@ -0,0 +1,83 @@
+/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- /
+/* vim: set shiftwidth=2 tabstop=2 autoindent cindent expandtab: */
+
+// Custom voicemail notification -- This can be removed once DesktopNotification
+// supports removing notifications via API
+var Voicemail = {
+
+ icon: null,
+ notification: null,
+ // A random starting point that is unlikely to be used by other notifications
+ notificationId: 3000 + Math.floor(Math.random() * 999),
+
+ init: function vm_init() {
+ window.navigator.mozApps.getSelf().onsuccess = (function(event) {
+ var app = event.target.result;
+ this.icon = app.installOrigin + '/style/statusbar/images/voicemail.png';
+ }).bind(this);
+ },
+
+ updateNotification: function vm_updateNotification(status) {
+ var _ = window.navigator.mozL10n.get;
+ var title = status.returnMessage;
+ var showCount = status.hasMessages && status.messageCount > 0;
+
+ if (!title) {
+ title = showCount ? _('newVoicemails', { n: status.messageCount }) :
+ _('newVoicemailsUnknown');
+ }
+
+ var text = title;
+ var voicemailNumber = navigator.mozVoicemail.number;
+ if (voicemailNumber) {
+ text = _('dialNumber', { number: voicemailNumber });
+ }
+
+ this.hideNotification();
+ if (status.hasMessages) {
+ this.showNotification(title, text, voicemailNumber);
+ }
+ },
+
+ showNotification: function vm_showNotification(title, text, voicemailNumber)
+ {
+ this.notificationId++;
+ this.notification = NotificationScreen.addNotification({
+ id: this.notificationId, title: title, text: text, icon: this.icon
+ });
+
+ if (!voicemailNumber) {
+ return;
+ }
+
+ var self = this;
+ function vmNotification_onTap(event) {
+ self.notification.removeEventListener('tap', vmNotification_onTap);
+
+ var telephony = window.navigator.mozTelephony;
+ if (!telephony) {
+ return;
+ }
+
+ telephony.dial(voicemailNumber);
+ }
+
+ this.notification.addEventListener('tap', vmNotification_onTap);
+ },
+
+ hideNotification: function vm_hideNotification() {
+ if (!this.notification) {
+ return;
+ }
+
+ if (this.notification.parentNode) {
+ NotificationScreen.removeNotification(this.notificationId);
+ }
+
+ this.notification = null;
+ this.notificationId = 0;
+ },
+
+};
+
+Voicemail.init();
View
5 apps/system/locales/system.en-US.properties
@@ -9,6 +9,11 @@ unreadMessages={[ plural(n) ]}
unreadMessages[zero] = You have no unread messages
unreadMessages[one] = You have one unread message
unreadMessages[other] = You have {{n}} unread messages
+newVoicemails={[ plural(n) ]}
+newVoicemails[one] = You have one new voicemail message
+newVoicemails[other] = You have {{n}} new voicemail messages
+newVoicemailsUnknown = You have new voicemail message(s)
+dialNumber = Dial {{number}}
missedCalls={[ plural(n) ]}
@etiennesegonzac Collaborator

Kudos on adding the new strings!

@marshall Owner

Will we need a translator to add these the other locales as well?

@etiennesegonzac Collaborator

The answer for this type of questions is to ping @fabi1cazenave :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
missedCalls[zero] = You have no missed calls
missedCalls[one] = You have one missed call
View
4 apps/system/manifest.webapp
@@ -11,7 +11,9 @@
"power",
"mozApps",
"mobileconnection",
- "mozBluetooth"
+ "mozBluetooth",
+ "telephony",
+ "voicemail"
],
"locales": {
"ar": {
View
BIN  apps/system/style/statusbar/images/voicemail-statusbar.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
View
BIN  apps/system/style/statusbar/images/voicemail.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
View
29 apps/system/style/statusbar/statusbar.css
@@ -43,6 +43,35 @@
background: url('images/loading.gif') center center repeat-x;
}
+#statusbar-voicemail {
+ width: 40px;
+ position: relative;
+}
+
+#statusbar-voicemail[data-show-num="true"]::before {
+ content: attr(data-num);
+ position: absolute;
+ color: #666;
+ width: 24px;
+ height: 16px;
+ top: 0;
+ left: 0;
+ font: 12px/12px 'Open Sans', sans-serif;
+ font-weight: 600;
+ text-align: right;
+}
+
+#statusbar-voicemail::after {
+ content: '';
+ position: absolute;
+ width: 16px;
+ height: 16px;
+ top: 0;
+ right: 0;
+ background: url('images/voicemail-statusbar.png') no-repeat;
+ background-position: center -2px;
+}
+
#statusbar-time {
padding: 0;
color: #fff;
View
4 build/preferences.js
@@ -91,6 +91,10 @@ let permissions = {
"urls": [],
"pref": "dom.mozBluetooth.whitelist"
},
+ "voicemail": {
+ "urls": [],
+ "pref": "dom.voicemail.whitelist"
@etiennesegonzac Collaborator

Out of curiosity, is it the same whitelist mechanism than WebTelephony (ie full URLs) or is it just on an origin base?

@marshall Owner

In my implementation I ensured it was origin based for simplicity (ala MobileNetwork etc)

@etiennesegonzac Collaborator

Origin based is good! Thanks for the info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ },
"mozbrowser": {
"urls": [],
"pref": "dom.mozBrowserFramesWhitelist"
Something went wrong with that request. Please try again.