Closed
Bug 813689
Opened 12 years ago
Closed 12 years ago
[SMS] Characters counter in SMS application
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect, P1)
Firefox OS Graveyard
Gaia::SMS
Tracking
(blocking-basecamp:+)
People
(Reporter: brg, Assigned: vingtetun)
References
Details
(Keywords: feature, Whiteboard: [target:12/21], UX-P1)
Attachments
(1 file)
(deleted),
patch
|
kaze
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121024073032
Steps to reproduce:
there is no information of how huge is the SMS or how many more characters I can type before having to send two SMS.
Actual results:
user should know when the length of the SMS is close to the max size.
Updated•12 years ago
|
Component: General → Gaia::SMS
Reporter | ||
Updated•12 years ago
|
blocking-basecamp: --- → ?
Updated•12 years ago
|
Priority: -- → P3
Whiteboard: UX_QA
Updated•12 years ago
|
Assignee: nobody → fbsc
blocking-basecamp: ? → +
Priority: P3 → P1
Comment 1•12 years ago
|
||
Mass Modify: All un-milestoned, unresolved blocking-basecamp+ bugs are being moved into the C3 milestone. Note that the target milestone does not mean that these bugs can't be resolved prior to 12/10, rather C2 bugs should be prioritized ahead of C3 bugs.
Target Milestone: --- → B2G C3 (12dec-1jan)
Updated•12 years ago
|
Whiteboard: UX_QA → UX_QA [target:12/21]
Comment 2•12 years ago
|
||
Why are we blocking in this? This is not in the original requirements, and doesn't seem like something we'd hold the release for.
blocking-basecamp: + → ?
Keywords: feature
Comment 3•12 years ago
|
||
This missed the feature complete milestone by a few months. Its a feature we can live without. We can put it on the v1.1 train. If this is a P1 requirement, and we decide to re-open the feature scope, we also would have to re-open the schedule. As a result I think this is b- in my opinion. Daniel, can you confirm as well please.
Flags: needinfo?(dcoloma)
Comment 4•12 years ago
|
||
Andreas, Dietrich: I've sent you a private e-mail about this.
Flags: needinfo?(dcoloma)
Comment 5•12 years ago
|
||
Necessary for certification requirements. Marking as blocking.
blocking-basecamp: ? → +
Comment 6•12 years ago
|
||
Hi Casey,
please ref https://bugzilla.mozilla.org/show_bug.cgi?id=813682#c7, thanks
Flags: needinfo?(kyee)
Updated•12 years ago
|
Assignee: fbsc → schung
Flags: needinfo?(kyee) → needinfo?(aymanmaat)
Comment 9•12 years ago
|
||
UX architecture to feedback to users on how many more characters can be typed before having to send another SMS is contained in Bug 813682
RFI to Steve Chung - should we merge these bugs?
Flags: needinfo?(aymanmaat) → needinfo?(schung)
Reporter | ||
Comment 10•12 years ago
|
||
(In reply to ayman maat :maat from comment #9)
> UX architecture to feedback to users on how many more characters can be
> typed before having to send another SMS is contained in Bug 813682
>
> RFI to Steve Chung - should we merge these bugs?
I see your point, you will fix 2 issues at the same time but the issues are different, I do not want to see them merged... because they are two different issues/things/features.
Comment 11•12 years ago
|
||
Hi,
I include this issue as a certification blocker for v1.
Thx!
David
Blocks: b2g-v1-certification
Comment 12•12 years ago
|
||
(In reply to Beatriz Rodríguez [:brg] from comment #10)
> (In reply to ayman maat :maat from comment #9)
> > UX architecture to feedback to users on how many more characters can be
> > typed before having to send another SMS is contained in Bug 813682
> >
> > RFI to Steve Chung - should we merge these bugs?
>
> I see your point, you will fix 2 issues at the same time but the issues are
> different, I do not want to see them merged... because they are two
> different issues/things/features.
I lean more toward Beatriz. Even these fixing would be fixed in one patch, it would be better to trace them independently. These counter/indicator returned by API are separated, we will not compute these numbers in gaia layer.
Flags: needinfo?(schung)
Comment 14•12 years ago
|
||
Will this counter also include the number of SMS which will be send out when the user hits send? Means how do we support multi-part SMS?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 15•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #14)
> Will this counter also include the number of SMS which will be send out when
> the user hits send? Means how do we support multi-part SMS?
yes. please refer to attachment of Bug 813682
Assignee | ||
Comment 16•12 years ago
|
||
Kaze has already looked at operator variant for 7bit-encoding so let's ask him the review since the code does not really alter any existing sms code but just add a very isolated method.
Assignee: schung → 21
Attachment #693927 -
Flags: review?(kaze)
Comment 17•12 years ago
|
||
Comment on attachment 693927 [details] [diff] [review]
Patch
Review of attachment 693927 [details] [diff] [review]:
-----------------------------------------------------------------
r=me for the code.
UX-wise, I’m a bit confused that we only display the number of allowed characters once that we’re *over* the one-message limit. I would expect to be warned that I’m going to get over the one-message limit *before* reaching it…
Josh, if the current behavior is not OK for you, please open a follow-up.
Attachment #693927 -
Flags: review?(kaze) → review+
Updated•12 years ago
|
Flags: needinfo?(jcarpenter)
Assignee | ||
Comment 18•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 19•12 years ago
|
||
(In reply to Fabien Cazenave [:kaze] from comment #17)
> Comment on attachment 693927 [details] [diff] [review]
> Patch
>
> Review of attachment 693927 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> r=me for the code.
>
> UX-wise, I’m a bit confused that we only display the number of allowed
> characters once that we’re *over* the one-message limit. I would expect to
> be warned that I’m going to get over the one-message limit *before* reaching
> it…
>
> Josh, if the current behavior is not OK for you, please open a follow-up.
specified behavior equalizes android UX. further justification can be found in comment 13 of bug 813682.
Comment 20•12 years ago
|
||
(In reply to ayman maat :maat from comment #19)
> specified behavior equalizes android UX. further justification can be found
> in comment 13 of bug 813682.
Hm, that's not fully true. Android shows the counter in the first message once the user crosses the 10 left character limit. That helps in adjusting the message so it will fit into a single one.
Comment 21•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [away 12/21 - 01/01] from comment #20)
> (In reply to ayman maat :maat from comment #19)
> > specified behavior equalizes android UX. further justification can be found
> > in comment 13 of bug 813682.
>
> Hm, that's not fully true. Android shows the counter in the first message
> once the user crosses the 10 left character limit. That helps in adjusting
> the message so it will fit into a single one.
My mistake. I was referring to a different OS when i wrote this comment. Spinning one too many plates at once there...
Comment 22•12 years ago
|
||
(In reply to Fabien Cazenave [:kaze] from comment #17)
> Josh, if the current behavior is not OK for you, please open a follow-up.
I think it's a valid request and went ahead. Bug 823437 has been filed which will cover that remaining issue.
Comment 23•12 years ago
|
||
I defer to Ayman here, given his ownership of SMS UX. Sounds like we've managed to add this feature, despite the requirement surfacing so late. Nice work.
Flags: needinfo?(jcarpenter)
Updated•12 years ago
|
Whiteboard: UX_QA [target:12/21] → [target:12/21], UX-P1
Comment 24•12 years ago
|
||
This one is not working properly (at least there is no indicator in today's build). Following https://bugzilla.mozilla.org/show_bug.cgi?id=813682#c16 we must reopen this one and probably backout the changes, because the implementation is not using the methods available in Gecko. This is so important because this is part of certification requirements should be treated as high priority.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 25•12 years ago
|
||
(In reply to Borja Salguero [:borjasalguero] from comment #24)
> This one is not working properly (at least there is no indicator in today's
> build). Following https://bugzilla.mozilla.org/show_bug.cgi?id=813682#c16 we
It's not broken for me. But I wonder if you are seeing bug 823437. See comment 22.
Comment 26•12 years ago
|
||
Yep, but one thing is the 'visual' stuff, which probably is right, and other is the functionality. The thing is that when I was talking with Vicamo and Steve about how to implement the patch to this bug we stablished some functionalities to be used in Gaia, and I dont see any Gecko request in the code of the patch (https://bug813689.bugzilla.mozilla.org/attachment.cgi?id=693927)... so probably, despite it seems to be working fine, it's not. I need more info from Vicamo.
[:vicamo] What should we do here? Backout and create a new patch? Or the current one is valid?
Flags: needinfo?(vyang)
Comment 27•12 years ago
|
||
(Patch in GitHub is the following https://github.com/mozilla-b2g/gaia/pull/7091/files )
Comment 28•12 years ago
|
||
Hi Borja,
I think we can replace the logic inside updateCounter with the backend api. There is no need to revert the patch totally :)
Comment 29•12 years ago
|
||
Hi Steve!,
Ok, for me it's ok. Do you want to work on it? I dont know if you have some patch almost done for this issue. BTW I could work on it next week (I will come back from my Holidays following Monday!). Thanks for the info! ;)
Comment 30•12 years ago
|
||
Since https://bugzilla.mozilla.org/show_bug.cgi?id=820780 already passed all the reviews, it should be landed in this week. I'll apply the patch right after that and you can review it next Monday, so have a nice holiday!
Updated•12 years ago
|
Assignee: 21 → schung
Assignee | ||
Comment 31•12 years ago
|
||
(In reply to Steve Chung from comment #28)
> Hi Borja,
> I think we can replace the logic inside updateCounter with the backend api.
> There is no need to revert the patch totally :)
What Steve says. Also let's open a new bug explaining what to change instead of reopening old ones.
Assignee: schung → 21
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Flags: needinfo?(vyang)
Comment 32•12 years ago
|
||
Refer to this bug for Further information
https://bugzilla.mozilla.org/show_bug.cgi?id=823437
Unagi Build 20130103070201
Comment 33•12 years ago
|
||
This issue is fixed for device Unagi; build #20130113070202v.1, sms character count shows a the start of second page
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•