Closed
Bug 1015194
Opened 11 years ago
Closed 8 years ago
[Messages] Invalid recipients are sometimes still considered as valid recipients
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect, P1)
Tracking
(tracking-b2g:backlog, b2g18 affected, b2g-v1.2 affected, b2g-v1.3 affected, b2g-v1.3T affected, b2g-v1.4 affected, b2g-v2.0 affected)
RESOLVED
WONTFIX
tracking-b2g | backlog |
People
(Reporter: julienw, Assigned: julienw)
References
Details
(Whiteboard: [sms-most-wanted])
Attachments
(1 file)
STR:
1. enter 3 invalid recipients (type: "a;" then "b;" then "c;" => this will enter 3 invalid recipients a, b, c)
2. tap recipient "a" => it will move to "editable" mode
3. tap recipient "b" => recipient "a" will move back to "invalid" and "b" will move to "editable"
=> the header reads "1 recipient" but it should read "new message"
4. enter some text
=> "send" button is enabled whereas it should be disabled
5. press send
=> we're moved to a "a" thread and a message is being sent; but it should not.
=> eventually we have an error.
We found the issue while working on bug 1011573.
This is probably happening since v1.1.
Assignee | ||
Comment 1•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #0)
> => eventually we have an error.
Given there is a letter-to-number conversion, it could happen to have a real number and have a real SMS sent. That said, someone is really trying to find bugs if that happens ;)
The issue comes from the fact we don't update the list if there is something currently typed (see [1]). This check is necessary because we don't really know the right index to change, we only guess it should be the last one, which is right in most cases, but not here.
As a result, the "to be invalid" recipient is never marked as invalid in the JS object, but it is _visually_ invalid because the class is correctly set, because it was invalid before.
It's not _that_ easy to fix it, it's also not _that_ difficult.
Assignee | ||
Comment 2•11 years ago
|
||
I've nominated it to 1.4? but feel free to block to 1.3t if necessary.
Assignee | ||
Updated•11 years ago
|
status-b2g18:
--- → affected
status-b2g-v1.2:
--- → affected
status-b2g-v1.3:
--- → affected
status-b2g-v1.3T:
--- → affected
status-b2g-v1.4:
--- → affected
status-b2g-v2.0:
--- → affected
Comment 3•11 years ago
|
||
Just noticed that it's also possible to send SMS to invalid recipient using the following STR:
1. Enter any message text;
2. Enter valid recipient and press enter (eg. "123");
3. Observe that send button is enabled;
4. Enter some invalid number (eg. "..."), but don't confirm it via Enter;
5. Tap on Send button.
Expected result: message is sent to "123" only;
Actual result: message is sent to both "123" and "...".
Comment 5•11 years ago
|
||
Paul,
Would this be a security issue? Are we opening a loop hole here?
Flags: needinfo?(ptheriault)
Assignee | ||
Comment 6•11 years ago
|
||
IMO this is no security issue, only a buggy behavior.
(adding a whiteboard so that the sms team can find it back when planning sprints)
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Whiteboard: [sms-most-wanted]
Comment 7•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] (away until 2nd June) from comment #6)
> IMO this is no security issue, only a buggy behavior.
> (adding a whiteboard so that the sms team can find it back when planning
> sprints)
Agreed.
Flags: needinfo?(ptheriault)
Assignee | ||
Comment 8•11 years ago
|
||
This was fixed in the v1.3t branch only in bug 989600 and bug 1011573. Should not be difficult to retrofit the fix here.
Assignee | ||
Comment 9•10 years ago
|
||
Taking to try to do what I suggested in comment 8.
Assignee: nobody → felash
Updated•10 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
Comment 11•10 years ago
|
||
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8592135 [details] [review]
[gaia] julienw:1015194-fix-recipients-handling > mozilla-b2g:master
just a WIP
Assignee | ||
Comment 13•10 years ago
|
||
Comment on attachment 8592135 [details] [review]
[gaia] julienw:1015194-fix-recipients-handling > mozilla-b2g:master
Hey Oleg,
what do you think of this approach ?
Known issue:
* the focus is not retained after I enter a recipient. I didn't find a good way to make it work yet, so if you have an idea I'd be glad to know it.
* we send several 'add' and render several times when we recover a draft; we could wait until all "add" operations have been handled, but I'm not sure the added complexity is worth it?
* I think we could remove some useless render/focus calls that happen in the event handler (in the view). What do you think?
Attachment #8592135 -
Flags: feedback?(azasypkin)
Assignee | ||
Comment 14•10 years ago
|
||
mmm and currently the STR in comment 0 is not working properly (one of the invalid recipients disappear :/).
Just let me know what you think of the approach !
Updated•10 years ago
|
Priority: -- → P1
Comment 15•10 years ago
|
||
Comment on attachment 8592135 [details] [review]
[gaia] julienw:1015194-fix-recipients-handling > mozilla-b2g:master
I think the approach is good, left some thoughts at GitHub as well.
Thanks!
Attachment #8592135 -
Flags: feedback?(azasypkin) → feedback+
Assignee | ||
Comment 16•8 years ago
|
||
Mass closing of Gaia::SMS bugs. End of an era :(
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
Assignee | ||
Comment 17•8 years ago
|
||
Mass closing of Gaia::SMS bugs. End of an era :(
You need to log in
before you can comment on or make changes to this bug.
Description
•