Closed Bug 497195 Opened 11 years ago Closed 11 years ago

Mobile: SMS feature

Categories

(www.mozilla.org :: General, defect, major)

defect
Not set
major

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: rdoherty, Assigned: davedash)

References

Details

(Whiteboard: [waiting on IT to activate our third-party SMS, before we can test])

Attachments

(2 files, 2 obsolete files)

Tracking creating of desktop Fennec sms feature. Splitting it out into a separate bug due to size and scope.
Attached patch v1 (obsolete) — Splinter Review
This patch depends on the approval of 497185's patch.  So if 497185 is not approved, then don't bother looking at this - I will fix 497185 and repatch this.

Also note, this is my first attempt at using git-svn if this patch doesn't work, I blame jbalogh :)
Attachment #384183 - Flags: review?(rdoherty)
> Also note, this is my first attempt at using git-svn if this patch doesn't
> work, I blame jbalogh :)

A good policy, regardless
Attached patch v2 (obsolete) — Splinter Review
Attachment #384183 - Attachment is obsolete: true
Attachment #384529 - Flags: review?(rdoherty)
Attachment #384183 - Flags: review?(rdoherty)
Comment on attachment 384529 [details] [diff] [review]
v2

Woo, I got a text message! It works pretty fast too.

Pretty minor nitpicks. Probably would be better to have status=success instead of 'notfail'. Double negatives can make things confusing.

I think automatically add '+1' to the phone field will annoy international users. Would it be better to do some JS detection to see if they didn't enter a country code? 

Also, if I enter 'asdkfjlds' into the phone # field, I get status=notfail. We should do basic validation on the backend of the phone #.
Attachment #384529 - Flags: review?(rdoherty) → review-
Attached patch v3Splinter Review
Will include some images as well in another attachment.
Attachment #384529 - Attachment is obsolete: true
Attachment #384904 - Flags: review?(rdoherty)
Duplicate of this bug: 500225
Comment on attachment 384904 [details] [diff] [review]
v3

Looks pretty good. I do see a 'notfail' in index.html and send.php, which I think should have been changed to 'success'?

The country list looks short to me, will want to make sure we don't forget any.
Attachment #384904 - Flags: review?(rdoherty) → review+
Hmm I think I need to change my patches that I submit to you... since they don't seem to fully apply.

If you look at the patch file itself, I did change those things.

I ganked the country list from Opera :)
host-7-214:~/Projects/Fennec/mozilla.com % git svn dcommit
Committing to https://svn.mozilla.org/projects/mozilla.com/trunk ...
	M	en-US/mobile/index.html
	A	en-US/mobile/send.php
	M	includes/config.inc.php-dist
	A	includes/messagemedia/SmsInterface.inc
	M	style/tignish/mobile.css
Committed r28412
	A	includes/messagemedia/SmsInterface.inc
	M	includes/config.inc.php-dist
	A	en-US/mobile/send.php
	M	en-US/mobile/index.html
	M	style/tignish/mobile.css
r28412 = 6e0a164a3f01a61a3b83b2ad860f473324f4b9e9 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
en-US/mobile/index.html: locally modified
en-US/mobile/send.php: locally modified
img/mobile/feature-box-background.png: locally modified
js/mobile-desktop.js: locally modified
style/tignish/mobile.css: locally modified
	M	en-US/mobile/index.html
	M	en-US/mobile/send.php
	M	img/mobile/feature-box-background.png
	M	js/mobile-desktop.js
	M	style/tignish/mobile.css
Committed r28413
	M	img/mobile/feature-box-background.png
	M	js/mobile-desktop.js
	M	en-US/mobile/send.php
	M	en-US/mobile/index.html
	M	style/tignish/mobile.css
r28413 = d72a61e80238c385d7b3744069a505369473a832 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
en-US/mobile/send.php: locally modified
	M	en-US/mobile/send.php
Committed r28414
	M	en-US/mobile/send.php
r28414 = bf639bf36f9bd27138f724c0b37d0b57187687c8 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
We're still waiting on IT in order to test SMS functionality.
Severity: normal → major
Whiteboard: [waiting on IT to activate our third-party SMS, before we can test]
Depends on: 499211
Component: www.mozilla.org/firefox → www.mozilla.org
Component: www.mozilla.org → General
Product: Websites → www.mozilla.org
You need to log in before you can comment on or make changes to this bug.