Closed
Bug 935060
Opened 11 years ago
Closed 8 years ago
[Messages] Unable to send empty message
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Firefox OS Graveyard
Gaia::SMS
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: Elio, Assigned: rishav_, Mentored)
References
Details
Attachments
(4 files, 6 obsolete files)
(deleted),
text/x-github-pull-request
|
Details | |
(deleted),
text/x-github-pull-request
|
Details | |
(deleted),
image/png
|
azasypkin
:
ui-review+
|
Details |
(deleted),
image/png
|
Details |
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20131105040203
Steps to reproduce:
Send an empty SMS to my operator in order to receive balance information
Actual results:
"Send" button disabled
Expected results:
"Send" button should be enabled also when no text is written.
The balance info for my operator for example works only if an empty SMS is sent to them, so Im unable to get any info from that
Comment 1•11 years ago
|
||
Flagging our UX designer Ayman on this.
I think the use case is sensible. Ayman, what do you think of this? It's your call!
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(aymanmaat)
Comment 2•11 years ago
|
||
Years ago we had the same debate in Maemo in https://bugs.maemo.org/show_bug.cgi?id=5409 and there are several reasons listed why it should be allowed to send empty SMS.
Comment 3•11 years ago
|
||
Thanks Andre, this is a very interesting discussion indeed. I agree we should present a warning message in this case.
Now it's Ayman's decision. :)
Comment 4•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #3)
> Thanks Andre, this is a very interesting discussion indeed. I agree we
> should present a warning message in this case.
>
> Now it's Ayman's decision. :)
(In reply to Julien Wajsberg [:julienw] from comment #3)
> Thanks Andre, this is a very interesting discussion indeed. I agree we
> should present a warning message in this case.
>
> Now it's Ayman's decision. :)
Hey guys, this is an interesting one, I had no idea about this. All the benchmarks i have access to reference do not allow the sending of a message when the message field is empty. However if this prevents a user from communicating with their operator (for example to obtain their balance information as outlined in comment 0 ) we should certainly allow the sending of empty messages.
If we are going to allow the send CTA to be active when there is no content in the message field we should certainly implement a warning message to handle cases where users select it by accident. Here is what i think:
**New Message**
1) open message app
2) open new message composer.
3) send CTA is disabled until content is entered into the ’to field’
4) user selects send with content in ’to field’ but message field empty
- warning dialogue is presented stating:
- <Header>Empty Message</Header>
- <Body>Do you want to send an empty message</Body>
- <CTA1>Cancel</CTA1><CTA2>OK</CTA2>
upon tap of CTA1 (Cancel)
- Warning dialogue is closed
- user is returned to view from which warning dialogue was launched.
upon tap of CTA2
- Warning dialogue is closed
- message is sent, flow follows currently implemented path
**Reply to Thread**
1) open message app
2) open existing thread.
3) send CTA is enabled
4) user selects send with message field empty
- warning dialogue is presented stating:
- <Header>Empty Message</Header>
- <Body>Do you want to send an empty message</Body>
- <CTA1>Cancel</CTA1><CTA2>OK</CTA2>
upon tap of CTA1 (Cancel)
- Warning dialogue is closed
- user is returned to view from which warning dialogue was launched.
upon tap of CTA2
- Warning dialogue is closed
- message is sent, flow follows currently implemented path
..how does that sound?
Flags: needinfo?(aymanmaat)
Comment 5•11 years ago
|
||
Sounds good to me.
1 question:
* Can we leave out the header? That would let us use the standard "window.confirm" function for an easy patch.
We should ask Tyler for good messages.
Flags: needinfo?(aymanmaat)
Reporter | ||
Comment 6•11 years ago
|
||
Any news on this?
Comment 7•11 years ago
|
||
Thanks for the heads up.
Just asked Ayman, he's fine with leaving out the header and we can use window.confirm then.
We just need a good message now. Tyler, could you please help? See comment 4, we need a good text for the "body" part.
Thanks!
Elio, do you think you could provide a patch for this or do you prefer that we do it?
Flags: needinfo?(aymanmaat) → needinfo?(tyler.altes)
Reporter | ||
Comment 8•11 years ago
|
||
Julien, feel free to do it, Im not that experienced with the tech part yet :)
Comment 9•11 years ago
|
||
Ok, no problem, we'll wait for Tyler's answer first :)
Whiteboard: [mentor=:julienw]
Comment 10•11 years ago
|
||
Hi, I think Ayman's message is fine, with the exception of punctuation. Then I would change the confirm button to correspond with the question.
- <Body>Do you want to send an empty message?</Body>
- <CTA1>Cancel</CTA1><CTA2>Send</CTA2>
Flags: needinfo?(tyler.altes)
Comment 11•11 years ago
|
||
For a contributor, the relevant code is in thread_ui.js (enableSend function and onSendClick function) and maybe in compose.js (in onContentChanged) too.
Comment 12•11 years ago
|
||
I am taking up these issue.
thanks Julien for the info!
I ll soon upload the required patch.
Comment 13•11 years ago
|
||
Kindly assign these issue to me as I am not able to assign it to myself.
Comment 14•11 years ago
|
||
Hi all.
I made a few changes to the code of thread_ui.js and compose.js to enable the send button for sending blank messages.
But the problem is that the mozMobileMessage.send function, when the message is empty, sends an "undefined" message.
Thus, I tried to used this function with an empty sms parameter (i.e content = ''), and the sms never send, I mean, there's a loading loop animation over the empty message.
Am I supposed to modify the mozMobileMessage.send function?
About the confirm button, can I get any help to do that?
Can you assign this to me? It would be great!
Thank you all for the information.
Cheers and happy new year!
Comment 15•11 years ago
|
||
Hi Nico and Ankit,
Thanks for your messages. Sorry for not answering earlier as I was in holiday.
Ankit, since I see no patch from you, is it ok if I assign the bug to Nico ?
Nico, the "confirm" panel should be done using the usual DOM0 "confirm()" function :) Yes, as simple as this!
About the issue with the "send" function, I'll need info gecko developers, as there may be a bug on their end. I think we're supposed to use send with "content = ''" but then either it should work or we should get an error.
Gene, could you provide some help on this topic?
Assignee: nobody → ndel314
Flags: needinfo?(gene.lian)
Comment 16•11 years ago
|
||
Hello!
First, thank you Julien for the assignment of this bug to me.
Some relevant issues are:
- Change "ok" to "send" of the confirmation panel
- Solve the problem of the empty message is never sent
I only changed the code in "onSendClick" and "enableSend" functions.
Maybe the part of "content[0] = '';" can be written in a smarter way; the Compose.clear() function doesn't do what I wanted.
Hope you can check this.
Regards!
Comment 17•11 years ago
|
||
Thank you for catching this!
> I made a few changes to the code of thread_ui.js and compose.js to enable
> the send button for sending blank messages.
> But the problem is that the mozMobileMessage.send function, when the message
> is empty, sends an "undefined" message.
One question. I don't understand this comment. Do you mean you set the |message| parameter to call the API:
jsval send(in jsval number, in DOMString message,
[optional] in jsval sendParameters);
to be an empty string (i.e., "") and the receiver end will receive a message "undefined"?
> Thus, I tried to used this function with an empty sms parameter (i.e content
> = ''), and the sms never send, I mean, there's a loading loop animation over
> the empty message.
I don't understand either. What's this different from the above-mentioned?
> Am I supposed to modify the mozMobileMessage.send function?
IMO, I think Gecko should be able to send empty string. If the sending circle keeps spinning, it means the Gecko doesn't correctly set the delivery state to "sent" [1]. Need to look into the starting point of sending SMS [2]. We may open a separate bug to address the Gecko part or just directly solved it in the same bug.
[1] http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/RadioInterfaceLayer.js#3487
[2] http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/RadioInterfaceLayer.js#3400
Flags: needinfo?(gene.lian)
Comment 18•11 years ago
|
||
Hey Julien
I ll upload the patch today..
Comment 19•11 years ago
|
||
Hello Gene!
> One question. I don't understand this comment. Do you mean you set the
> |message| parameter to call the API:
>
> jsval send(in jsval number, in DOMString message,
> [optional] in jsval sendParameters);
>
> to be an empty string (i.e., "") and the receiver end will receive a message
> "undefined"?
The first thing I did was to enable the "send" button for empty messages case, and what I got after sending the first blank message was that message isn't empty but "undefined".
> I don't understand either. What's this different from the above-mentioned?
Yep, I didn't explained that well.
The difference is that the above-mentioned is not "empty" in a way, because the send function acts different between both. I mean, when I used the function send with a blank message (i.e, ''), sends an empty message but the circle keeps spinning, and the another case sends an message with "undefined" word in it.
I think the Compose.getContent() function lefts something more than "''" when the message is empty.
I'll check it out.
> > Am I supposed to modify the mozMobileMessage.send function?
>
> IMO, I think Gecko should be able to send empty string. If the sending
> circle keeps spinning, it means the Gecko doesn't correctly set the delivery
> state to "sent" [1]. Need to look into the starting point of sending SMS
> [2]. We may open a separate bug to address the Gecko part or just directly
> solved it in the same bug.
I agree with that.
> [1]
> http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/
> RadioInterfaceLayer.js#3487
> [2]
> http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/
> RadioInterfaceLayer.js#3400
You could see how the code that I'll uploaded works?
I'll make a patch then.
Cheers!
Comment 20•11 years ago
|
||
The "undefined" case comes probably from WebIDL: if the message body is undefined (as in "the variable is undefined"), then WebIDL will convert it to a String "undefined". And that's why we get "undefined" as text.
I've seen that when working on the Contacts API, but that's always been weird to me. I think the only sensible ways to work with this in WebIDL is either to throw if we don't have a String, accept undefined and null as real values, or treat undefined as the empty string. The default behavior is very wrong to me.
Comment 21•11 years ago
|
||
This is the patch of the first attempt to resolve the #935060 bug.
Comment 22•11 years ago
|
||
Comment on attachment 8356166 [details] [diff] [review]
bug-935060-fix1 First attempt patch
Review of attachment 8356166 [details] [diff] [review]:
-----------------------------------------------------------------
A few general notes:
- Please make sure all of the current tests pass
- Please add new tests for the new functionality
- Please be sure to run both `make hint APP=sms` and `make lint APP=sms` (there are currently 6 known errors, your code should introduce no new errors)
::: thread_ui.js
@@ +1931,2 @@
> return;
> + }*/
If this is no longer needed, please remove it entirely
@@ +1957,5 @@
>
> + // If it's an empty message, the content is '' (bug 935060)
> + if (Compose.isEmpty()) {
> + content[0] = '';
> + if(!window.confirm('Do you want to send an empty message?'))
This won't work with l10n
@@ +1958,5 @@
> + // If it's an empty message, the content is '' (bug 935060)
> + if (Compose.isEmpty()) {
> + content[0] = '';
> + if(!window.confirm('Do you want to send an empty message?'))
> + return;
nit: there is a space missing between if and (.
Please always use curly braces for conditional bodies (or while, for etc)
Attachment #8356166 -
Flags: review-
Comment 23•11 years ago
|
||
Also, please r? me when ready, thanks!
Comment 24•11 years ago
|
||
Note that we'll need a Gecko patch before merging this. Gene, can you file a new bug and produce a patch to fix the "empty string as body" case?
Flags: needinfo?(gene.lian)
Comment 25•11 years ago
|
||
Comment on attachment 8356166 [details] [diff] [review]
bug-935060-fix1 First attempt patch
Review of attachment 8356166 [details] [diff] [review]:
-----------------------------------------------------------------
Please request review from Rick when you've finished!
Also, please open a github pull request so that a Travis jobs are run.
::: thread_ui.js
@@ +1957,5 @@
>
> + // If it's an empty message, the content is '' (bug 935060)
> + if (Compose.isEmpty()) {
> + content[0] = '';
> + if(!window.confirm('Do you want to send an empty message?'))
l10n means localization. Have a look to the uses of "navigator.mozL10n.get" in the code.
@@ +1958,5 @@
> + // If it's an empty message, the content is '' (bug 935060)
> + if (Compose.isEmpty()) {
> + content[0] = '';
> + if(!window.confirm('Do you want to send an empty message?'))
> + return;
also please add blocks even for single-line "if" :)
Updated•11 years ago
|
Whiteboard: [mentor=:julienw] → [mentor=rwaldron]
Comment 27•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #25)
> Comment on attachment 8356166 [details] [diff] [review]
> bug-935060-fix1 First attempt patch
>
> Review of attachment 8356166 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Please request review from Rick when you've finished!
>
> Also, please open a github pull request so that a Travis jobs are run.
>
> ::: thread_ui.js
> @@ +1957,5 @@
> >
> > + // If it's an empty message, the content is '' (bug 935060)
> > + if (Compose.isEmpty()) {
> > + content[0] = '';
> > + if(!window.confirm('Do you want to send an empty message?'))
>
> l10n means localization. Have a look to the uses of "navigator.mozL10n.get"
> in the code.
Unfortunately, `navigator.mozL10n.get` will fail if the user changes the language while the app is open.
Tell me what you think:
1. Somewhere in index.html, we'll have something like this:
<span id="confirm-empty-message" data-l10n-id="confirm-empty-message" class="hidden">Do you want to send an empty message?</span>
2. Add an entry in locales: confirm-empty-message = Do you want to send an empty message?
3. During ThreadUI.init or Compose.init, call `navigator.mozL10n.localize(element, element.dataset.l10nId);`
4. Update the confirm call to: `!window.confirm(document.getElementById('confirm-empty-message').textContent)`
NOTE: This wasn't tested and is only meant to kickstart discussion for an alternative to using `navigator.mozL10n.get`. We could also abstract the operation to provide l10n handling to all of the remaining uses of `navigator.mozL10n.get` as well.
Updated•11 years ago
|
Summary: Unable to send empty SMS' → [Messages] Unable to send empty message
Comment 28•11 years ago
|
||
(In reply to Rick Waldron [:rwaldron] from comment #27)
> (In reply to Julien Wajsberg [:julienw] from comment #25)
> > Comment on attachment 8356166 [details] [diff] [review]
> > bug-935060-fix1 First attempt patch
> >
> > Review of attachment 8356166 [details] [diff] [review]:
> > -----------------------------------------------------------------
> >
> > Please request review from Rick when you've finished!
> >
> > Also, please open a github pull request so that a Travis jobs are run.
> >
> > ::: thread_ui.js
> > @@ +1957,5 @@
> > >
> > > + // If it's an empty message, the content is '' (bug 935060)
> > > + if (Compose.isEmpty()) {
> > > + content[0] = '';
> > > + if(!window.confirm('Do you want to send an empty message?'))
> >
> > l10n means localization. Have a look to the uses of "navigator.mozL10n.get"
> > in the code.
>
> Unfortunately, `navigator.mozL10n.get` will fail if the user changes the
> language while the app is open.
>
> Tell me what you think:
>
> 1. Somewhere in index.html, we'll have something like this:
> <span id="confirm-empty-message" data-l10n-id="confirm-empty-message"
> class="hidden">Do you want to send an empty message?</span>
> 2. Add an entry in locales: confirm-empty-message = Do you want to send an
> empty message?
> 3. During ThreadUI.init or Compose.init, call
> `navigator.mozL10n.localize(element, element.dataset.l10nId);`
> 4. Update the confirm call to:
> `!window.confirm(document.getElementById('confirm-empty-message').
> textContent)`
>
> NOTE: This wasn't tested and is only meant to kickstart discussion for an
> alternative to using `navigator.mozL10n.get`. We could also abstract the
> operation to provide l10n handling to all of the remaining uses of
> `navigator.mozL10n.get` as well.
I fixed that issue using `navigator.mozL10n.get` adding to each language `emptyMessage-confirmation = Do you want to send an empty message?` (with translation for each language, in sms.lang.properties).
This allows me to use mozL10n.get like this way:
var question = navigator.mozL10n.get('emptyMessage-confirmation');
if (!window.confirm(question)) {
return;
}
And this worked fine.
The problem with this solution is that the button is "OK" instead of "Send".
Any way to change that?
I'll test your idea. And tell me your opinion of this solution.
Regards.
Comment 29•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #25)
> Comment on attachment 8356166 [details] [diff] [review]
> bug-935060-fix1 First attempt patch
>
> Review of attachment 8356166 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Please request review from Rick when you've finished!
>
> Also, please open a github pull request so that a Travis jobs are run.
>
> ::: thread_ui.js
> @@ +1957,5 @@
> >
> > + // If it's an empty message, the content is '' (bug 935060)
> > + if (Compose.isEmpty()) {
> > + content[0] = '';
> > + if(!window.confirm('Do you want to send an empty message?'))
>
> l10n means localization. Have a look to the uses of "navigator.mozL10n.get"
> in the code.
>
> @@ +1958,5 @@
> > + // If it's an empty message, the content is '' (bug 935060)
> > + if (Compose.isEmpty()) {
> > + content[0] = '';
> > + if(!window.confirm('Do you want to send an empty message?'))
> > + return;
>
> also please add blocks even for single-line "if" :)
Hey Julien,
I fixed all these errors and I'm familiarizing with the rules :D.
But before reply to Rick and upload the patch again, I prefer to finish the Gecko part.
Comment 30•11 years ago
|
||
(In reply to ndel314 from comment #28)
> (In reply to Rick Waldron [:rwaldron] from comment #27)
> > (In reply to Julien Wajsberg [:julienw] from comment #25)
> > > Comment on attachment 8356166 [details] [diff] [review]
> > > bug-935060-fix1 First attempt patch
> > >
> > > Review of attachment 8356166 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > >
> > > Please request review from Rick when you've finished!
> > >
> > > Also, please open a github pull request so that a Travis jobs are run.
> > >
> > > ::: thread_ui.js
> > > @@ +1957,5 @@
> > > >
> > > > + // If it's an empty message, the content is '' (bug 935060)
> > > > + if (Compose.isEmpty()) {
> > > > + content[0] = '';
> > > > + if(!window.confirm('Do you want to send an empty message?'))
> > >
> > > l10n means localization. Have a look to the uses of "navigator.mozL10n.get"
> > > in the code.
> >
> > Unfortunately, `navigator.mozL10n.get` will fail if the user changes the
> > language while the app is open.
> >
> > Tell me what you think:
> >
> > 1. Somewhere in index.html, we'll have something like this:
> > <span id="confirm-empty-message" data-l10n-id="confirm-empty-message"
> > class="hidden">Do you want to send an empty message?</span>
> > 2. Add an entry in locales: confirm-empty-message = Do you want to send an
> > empty message?
> > 3. During ThreadUI.init or Compose.init, call
> > `navigator.mozL10n.localize(element, element.dataset.l10nId);`
> > 4. Update the confirm call to:
> > `!window.confirm(document.getElementById('confirm-empty-message').
> > textContent)`
> >
> > NOTE: This wasn't tested and is only meant to kickstart discussion for an
> > alternative to using `navigator.mozL10n.get`. We could also abstract the
> > operation to provide l10n handling to all of the remaining uses of
> > `navigator.mozL10n.get` as well.
>
> I fixed that issue using `navigator.mozL10n.get` adding to each language
> `emptyMessage-confirmation = Do you want to send an empty message?` (with
> translation for each language, in sms.lang.properties).
In the future, you don't need to do this—it's handled by the l10n experts ;)
> This allows me to use mozL10n.get like this way:
>
> var question = navigator.mozL10n.get('emptyMessage-confirmation');
> if (!window.confirm(question)) {
> return;
> }
>
> And this worked fine.
If you change the language on the phone (while the application is running) is this string updated? (using your manually entered translations)
> The problem with this solution is that the button is "OK" instead of "Send".
Is there a spec for this? "OK" might be "OK" ;)
>
> Any way to change that?
Not for the standard `confirm` prompt.
>
Comment 31•11 years ago
|
||
Rick, yep, I agree, but I figured that on the fly translation for such transient dialogs is not so important, and does not warrant adding complexity to the app instead of using DOM0's confirm. I agree this is subjective though :)
Would you agree or would you rather do the in-app confirm dialog?
Comment 32•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #31)
> Rick, yep, I agree, but I figured that on the fly translation for such
> transient dialogs is not so important, and does not warrant adding
> complexity to the app instead of using DOM0's confirm. I agree this is
> subjective though :)
>
> Would you agree or would you rather do the in-app confirm dialog?
I agree with your rationale here, let's keep with the simple `confirm`
Comment 33•11 years ago
|
||
So, having fixed the Gecko part we get back here :).
> 1. Somewhere in index.html, we'll have something like this:
> <span id="confirm-empty-message" data-l10n-id="confirm-empty-message"
> class="hidden">Do you want to send an empty message?</span>
> 2. Add an entry in locales: confirm-empty-message = Do you want to send an
> empty message?
> 3. During ThreadUI.init or Compose.init, call
> `navigator.mozL10n.localize(element, element.dataset.l10nId);`
> 4. Update the confirm call to:
> `!window.confirm(document.getElementById('confirm-empty-message').
> textContent)`
Rick and Julien, this is the simple 'confirm' implementation that you discussed above?
Can I add the entry in locales directly? What would happen with the other languages?
If it's correct I'll start to implement it as soon as posible.
Regards!
Updated•11 years ago
|
Flags: needinfo?(waldron.rick)
Flags: needinfo?(felash)
Comment 34•11 years ago
|
||
(In reply to Nicolás Del Piano from comment #33)
> So, having fixed the Gecko part we get back here :).
>
> > 1. Somewhere in index.html, we'll have something like this:
> > <span id="confirm-empty-message" data-l10n-id="confirm-empty-message"
> > class="hidden">Do you want to send an empty message?</span>
> > 2. Add an entry in locales: confirm-empty-message = Do you want to send an
> > empty message?
> > 3. During ThreadUI.init or Compose.init, call
> > `navigator.mozL10n.localize(element, element.dataset.l10nId);`
> > 4. Update the confirm call to:
> > `!window.confirm(document.getElementById('confirm-empty-message').
> > textContent)`
>
> Rick and Julien, this is the simple 'confirm' implementation that you
> discussed above?
nope, we don't need the complex localize+textContent because, well, this doesn't actually bring any improvement :)
You just need to use navigator.mozL10n.get('confirm-empty-message') as confirm argument.
>
> Can I add the entry in locales directly? What would happen with the other
> languages?
Yep, the localizers will see the new key and will translate accordingly.
Flags: needinfo?(waldron.rick)
Flags: needinfo?(felash)
Comment 35•11 years ago
|
||
Hello,
I upload the patch.
Some issues:
I leave the part of 'confirm-empty-message' empty. So there is no message for now. It's ok this? Or have I to put something and leave this to the translators?
Attachment #8355733 -
Attachment is obsolete: true
Attachment #8356166 -
Attachment is obsolete: true
Flags: needinfo?(waldron.rick)
Comment 36•11 years ago
|
||
Yes, please add the new l10n-id with the text:
Do you want to send an empty message?
(see comment 10 :) )
Thanks!
Comment 37•11 years ago
|
||
Thanks Julien.
Sorry for the two patches. After the review I will mix them in one.
Flags: needinfo?(waldron.rick)
Comment 38•11 years ago
|
||
Comment on attachment 8369289 [details] [diff] [review]
Bug 935060 - locales text
look fine,
can you merge this into the other patch and do a github pull request with all these changes ?
You can attach a pull request on a bug by pasting directly the pull request URL (click "paste attachment as text" when attaching a new file).
And then ask review again :)
Thanks!
Attachment #8369289 -
Flags: feedback+
Comment 39•11 years ago
|
||
Thanks Julien!
Sorry for the delay...
Here is the full patch to be reviewed.
I don't know if I did the pull request, I attached the patch normally. What's the difference?
Attachment #8368939 -
Attachment is obsolete: true
Attachment #8369289 -
Attachment is obsolete: true
Flags: needinfo?(waldron.rick)
Flags: needinfo?(felash)
Comment 40•11 years ago
|
||
Attachment #8380430 -
Attachment is obsolete: true
Comment 41•11 years ago
|
||
(In reply to Nicolás Del Piano from comment #39)
> Created attachment 8380430 [details] [diff] [review]
> This patch enables the sending of empty messages.
>
> Thanks Julien!
>
> Sorry for the delay...
>
> Here is the full patch to be reviewed.
>
> I don't know if I did the pull request, I attached the patch normally.
> What's the difference?
The difference is that you'd push your working branch to github, make a pull request to mozilla-b2g/gaia and then paste that link as Julien describes.
Flags: needinfo?(waldron.rick)
Comment 42•11 years ago
|
||
Comment on attachment 8380431 [details] [diff] [review]
Bug 935060 - [Messages] Unable to send empty message patch.patch
Review of attachment 8380431 [details] [diff] [review]:
-----------------------------------------------------------------
::: apps/sms/js/thread_ui.js
@@ +912,5 @@
>
> enableSend: function thui_enableSend() {
> this.initSentAudio();
>
> // should disable if we have no message input
This comment needs to be updated
@@ +913,5 @@
> enableSend: function thui_enableSend() {
> this.initSentAudio();
>
> // should disable if we have no message input
> + var disableSendMessage = Compose.isResizing;
This change will have broken many tests—which means those tests need to be updated and run to ensure they aren't failing.
@@ +2072,5 @@
> }
>
> + // If it's an empty message, the content is ''
> + if (Compose.isEmpty()) {
> + content[0] = '';
Is this necessary?
@@ +2075,5 @@
> + if (Compose.isEmpty()) {
> + content[0] = '';
> + var question = navigator.mozL10n.get('confirm-empty-message');
> + if (!window.confirm(question)) {
> + return;
Please add two tests:
1. mock confirm to return true
2. mock confirm to return false
Comment 43•11 years ago
|
||
> >
> > + // If it's an empty message, the content is ''
> > + if (Compose.isEmpty()) {
> > + content[0] = '';
>
> Is this necessary?
Without that, if we attempt to send an empty message, it sends the word "undefined"; maybe some character remains on the message that makes this behavior.
What do you think about this?
> Please add two tests:
> 1. mock confirm to return true
> 2. mock confirm to return false
I'll check this.
Comment 44•11 years ago
|
||
(In reply to Nicolás Del Piano from comment #43)
> > >
> > > + // If it's an empty message, the content is ''
> > > + if (Compose.isEmpty()) {
> > > + content[0] = '';
> >
> > Is this necessary?
>
> Without that, if we attempt to send an empty message, it sends the word
> "undefined"; maybe some character remains on the message that makes this
> behavior.
>
> What do you think about this?
"'content.push('')" is probably more readable.
Also, please check how this behaves when you add a subject (both empty and non-empty) and this consequently converts to MMS (when non-empty). We might want to do "if (!content.length) { content.push(''); }" instead. And maybe move this to Compose.getContent (but in that case we need to check that other consulers like drafts still work fine).
Flags: needinfo?(felash)
Comment 45•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #44)
> (In reply to Nicolás Del Piano from comment #43)
> > > >
> > > > + // If it's an empty message, the content is ''
> > > > + if (Compose.isEmpty()) {
> > > > + content[0] = '';
> > >
> > > Is this necessary?
> >
> > Without that, if we attempt to send an empty message, it sends the word
> > "undefined"; maybe some character remains on the message that makes this
> > behavior.
> >
> > What do you think about this?
>
> "'content.push('')" is probably more readable.
>
> Also, please check how this behaves when you add a subject (both empty and
> non-empty) and this consequently converts to MMS (when non-empty). We might
> want to do "if (!content.length) { content.push(''); }" instead. And maybe
> move this to Compose.getContent (but in that case we need to check that
> other consulers like drafts still work fine).
Ok, there will be more clever if we change that on compose.js.
This is the isEmpty method of Compose class:
get isEmpty() {
return !dom.subject.value.length;
}
!content.length and content.isEmpty have the same result.
When trying to send an empty MMS, an error occurs:
"There is a problem with the attached file and it cannot be downloaded"
Should we may be able to send an empty MMS?
I think this problem is generated here:
var smilSlides = content.reduce(thui_generateSmilSlides, []);
Comment 46•11 years ago
|
||
Attachment #8380431 -
Attachment is obsolete: true
Comment 47•11 years ago
|
||
(In reply to Nicolás Del Piano from comment #45)
> (In reply to Julien Wajsberg [:julienw] from comment #44)
> > (In reply to Nicolás Del Piano from comment #43)
> > > > >
> > > > > + // If it's an empty message, the content is ''
> > > > > + if (Compose.isEmpty()) {
> > > > > + content[0] = '';
> > > >
> > > > Is this necessary?
> > >
> > > Without that, if we attempt to send an empty message, it sends the word
> > > "undefined"; maybe some character remains on the message that makes this
> > > behavior.
> > >
> > > What do you think about this?
> >
> > "'content.push('')" is probably more readable.
> >
> > Also, please check how this behaves when you add a subject (both empty and
> > non-empty) and this consequently converts to MMS (when non-empty). We might
> > want to do "if (!content.length) { content.push(''); }" instead. And maybe
> > move this to Compose.getContent (but in that case we need to check that
> > other consulers like drafts still work fine).
>
> Ok, there will be more clever if we change that on compose.js.
>
> This is the isEmpty method of Compose class:
>
> get isEmpty() {
> return !dom.subject.value.length;
> }
Nope, this is the "isEmpty" for the "subject" subobject :)
There is another "isEmpty" method for the whole "Compose" object ;)
>
> !content.length and content.isEmpty have the same result.
Not really, search for "state.empty" in the file.
>
> When trying to send an empty MMS, an error occurs:
> "There is a problem with the attached file and it cannot be downloaded"
Actually, this is misleading, because this is what's happening when we _display_ the MMS with an empty content.
That said, I really thought this didn't happen nowadays... Have you tried to send a MMS with a subject but no content (that is bug 948958, so you may want to rebase with a newer master if you're seeing this) or a MMS with empty subject and no content (which is not possible before your patch is done, and should not be a MMS) ?
>
> Should we may be able to send an empty MMS?
> I think this problem is generated here:
> var smilSlides = content.reduce(thui_generateSmilSlides, []);
I think this is not the problem here.
Comment 48•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #47)
> (In reply to Nicolás Del Piano from comment #45)
> > (In reply to Julien Wajsberg [:julienw] from comment #44)
> > > (In reply to Nicolás Del Piano from comment #43)
> > > > > >
> > > > > > + // If it's an empty message, the content is ''
> > > > > > + if (Compose.isEmpty()) {
> > > > > > + content[0] = '';
> > > > >
> > > > > Is this necessary?
> > > >
> > > > Without that, if we attempt to send an empty message, it sends the word
> > > > "undefined"; maybe some character remains on the message that makes this
> > > > behavior.
> > > >
> > > > What do you think about this?
> > >
> > > "'content.push('')" is probably more readable.
> > >
> > > Also, please check how this behaves when you add a subject (both empty and
> > > non-empty) and this consequently converts to MMS (when non-empty). We might
> > > want to do "if (!content.length) { content.push(''); }" instead. And maybe
> > > move this to Compose.getContent (but in that case we need to check that
> > > other consulers like drafts still work fine).
> >
> > Ok, there will be more clever if we change that on compose.js.
> >
> > This is the isEmpty method of Compose class:
> >
> > get isEmpty() {
> > return !dom.subject.value.length;
> > }
>
> Nope, this is the "isEmpty" for the "subject" subobject :)
>
> There is another "isEmpty" method for the whole "Compose" object ;)
>
> >
> > !content.length and content.isEmpty have the same result.
>
> Not really, search for "state.empty" in the file.
>
> >
> > When trying to send an empty MMS, an error occurs:
> > "There is a problem with the attached file and it cannot be downloaded"
>
> Actually, this is misleading, because this is what's happening when we
> _display_ the MMS with an empty content.
>
> That said, I really thought this didn't happen nowadays... Have you tried to
> send a MMS with a subject but no content (that is bug 948958, so you may
> want to rebase with a newer master if you're seeing this) or a MMS with
> empty subject and no content (which is not possible before your patch is
> done, and should not be a MMS) ?
>
> >
> > Should we may be able to send an empty MMS?
> > I think this problem is generated here:
> > var smilSlides = content.reduce(thui_generateSmilSlides, []);
>
> I think this is not the problem here.
Please, forget what I wrote above...
When I try to send an MMS with no content and some subject, it's sends normally.
> !content.length and content.isEmpty have the same result.
What I meant with this is that I had the same behavior when sending an empty message (It sends correctly).
You're right (I don't know why I got confused with that :S), the method "isEmpty" that returns "state.empty" is the one that are we looking for.
So, what is your opinion of "!content.lenght" instead of "content.isEmpty"?
Do you want to change the if condition directly, or do another thing?
I will see the adding of the mock tests :).
Comment 49•11 years ago
|
||
Discussed on IRC, "if (!content.length)" is the way to go!
Comment 50•11 years ago
|
||
Hey Nicolas, I wanted to make sure you're still working on this :)
Thanks !
Flags: needinfo?(ndel314)
Updated•10 years ago
|
Mentor: waldron.rick
Whiteboard: [mentor=rwaldron]
Comment 51•10 years ago
|
||
Hello, sorry for my late response.
The patch that I have submitted solves the issue, and I guess it has the minimal changes to fix the bug, however I need to write some mock test to it.
Anyone could give me a hint on this (i.e., what kind of tests have I to write)?
Also, how could I test it to see which tests are breaking?
Thanks!
Flags: needinfo?(waldron.rick)
Flags: needinfo?(ndel314)
Flags: needinfo?(felash)
Comment 52•10 years ago
|
||
The easiest is to follow what's in [1].
So basically:
* install firefox nightly
* set the FIREFOX environment variable to this install
* run bin/gaia-test (possibly with "-f" if you want to force creating a new profile)
* run (in a separate window): APP=sms make test-agent-test
Then you should see the unit tests that are failing (or maybe not).
The unit tests for the SMS app are located in the directory "apps/sms/test/unit", you'll see there is one test file per js file. You need to find the correct place to change here.
One of my rule is that every app change needs to have a unit test change :)
Don't hesitate to request some help if you need it, sharing your pull request :)
[1] https://developer.mozilla.org/en-US/Firefox_OS/Platform/Automated_testing/Gaia_unit_tests#Launching_the_test_runner_in_Firefox
Mentor: waldron.rick → felash
Flags: needinfo?(waldron.rick)
Flags: needinfo?(felash)
Assignee | ||
Comment 53•10 years ago
|
||
Hi Nicolas
Are you still working on this?
Thanks
Flags: needinfo?(ndel314)
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(rishav006)
Assignee | ||
Updated•10 years ago
|
Assignee: ndel314 → nobody
Flags: needinfo?(rishav006)
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(ndel314)
Assignee | ||
Comment 54•10 years ago
|
||
Hi Jenny
As ayaman opinion was very long ago, julien suggested me to discuss this with you.
What's your opinion on this? Do we need this feature?
Thanks
Flags: needinfo?(jelee)
Comment 55•10 years ago
|
||
Hi Kumar,
Given the recent change of 2.2 planning, the UX team will evaluate all feature and UI requests after v3 planning, so I'll revisit the topic until then, thanks for understanding!
Flags: needinfo?(jelee)
Comment 56•10 years ago
|
||
Hey Jenny,
I don't think we need to wait for v3 planning to answer the question for this simple feature request from a user. To make the story short: the user needs this feature in order to get balance from his operator. I definitely support doing this, but I'd like to have your stamp before doing so, in case you have serious concerns.
Thanks a lot :)
Flags: needinfo?(jelee)
Assignee | ||
Comment 57•10 years ago
|
||
Hi Julien
Is there any operator that gives balance info after sending empty message.(not talking about balance showed after reducing of message charge)
Thanks
Comment 58•10 years ago
|
||
This is what the reporter is saying in comment 0 :)
Assignee | ||
Comment 59•10 years ago
|
||
ni? Elio to give more detail on user story and on comment 57.
Thanks
Flags: needinfo?(ping)
Reporter | ||
Comment 60•10 years ago
|
||
Hi there!
Unfortunately I cant remember the details, but I remember I couldnt get my balance via SMS, due to this.
I have changed my plan and mobile phone now, so cant really replicate it as it has been a long time.
It was Eagle Mobile in Albania btw.
Flags: needinfo?(ping)
Comment 61•10 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #56)
> Hey Jenny,
>
> I don't think we need to wait for v3 planning to answer the question for
> this simple feature request from a user. To make the story short: the user
> needs this feature in order to get balance from his operator. I definitely
> support doing this, but I'd like to have your stamp before doing so, in case
> you have serious concerns.
>
> Thanks a lot :)
My only concern would be sending out accidental empty message that cost money, but since sending empty message has actual function, I don't think we should block it. Please go ahead and implement this feature ;) !
Flags: needinfo?(jelee)
Comment 63•10 years ago
|
||
Just to confirm relevance of this feature one more time :) I've seen some custom partner code recently. And they provide this feature with confirmation dialog to eliminate accidentally sent empty messages.
Comment 64•10 years ago
|
||
Hello,
I would like to work on this bug,
Please assign this bug to me,
Thank you.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → anusha91rao
Comment 65•10 years ago
|
||
See comment 52 for information about how to run unit tests, and note that there is a 'work in progress' patch attached to this bug too. So you might want to start from this patch, unless it's too old and does not work anymore.
Thanks!
Assignee | ||
Comment 66•9 years ago
|
||
Anusha, are you still working on this? Feel free to comment or come over irc, if you need any help.
Flags: needinfo?(anusha91rao)
Comment 67•9 years ago
|
||
Yes was caught up with something,
Will keep you updated .
Flags: needinfo?(anusha91rao)
Assignee | ||
Updated•9 years ago
|
Assignee: anusha91rao → nobody
Assignee | ||
Comment 68•9 years ago
|
||
Hi Anusha,
It's been long time, any update on the patch? ping on irc if you need any help?
Assigning to nobody so that other can pick it up :)
Thanks
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(rishav006)
Assignee | ||
Comment 69•9 years ago
|
||
Hey Julien,
This is long pending bug, want to work on this.
But before starting i need STR and few clarification (i read previous comments, but still want to confirm again, also discussion is too old).
1. what does it mean by empty message. Empty message == empty string i.e '' ?
2. What will receiver receive on the other end. Undefined or empty message or something else ?
It will be great if we get output from other device which support this functionality (i.e country/device/operator which support empty message to get balance info). (it will be great if we can have screenshot or video otherwise okay).
I am still afraid of gecko end. How it handle this. Will it allow this. Do we need gecko changes ?
[Discussion was very old, so i want to get update on it before starting :)]
Thanks
Flags: needinfo?(rishav006) → needinfo?(felash)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → rishav006
Comment 70•9 years ago
|
||
(In reply to kumar rishav (:rishav_) from comment #69)
> It will be great if we get output from other device which support this
> functionality (i.e country/device/operator which support empty message to
> get balance info).
See comment 2.
Assignee | ||
Comment 71•9 years ago
|
||
Yup, I saw that comment and related discussion too.
But what i was asking here is, what user will receive at other end. Let's say, i sent a empty message to my friend, what he will receive.
Thanks
Comment 72•9 years ago
|
||
(In reply to kumar rishav (:rishav_) from comment #69)
> Hey Julien,
>
> This is long pending bug, want to work on this.
> But before starting i need STR and few clarification (i read previous
> comments, but still want to confirm again, also discussion is too old).
>
> 1. what does it mean by empty message. Empty message == empty string i.e ''
> ?
Yes :)
> 2. What will receiver receive on the other end. Undefined or empty message
> or something else ?
Empty message !
>
> It will be great if we get output from other device which support this
> functionality (i.e country/device/operator which support empty message to
> get balance info). (it will be great if we can have screenshot or video
> otherwise okay).
I hope you can do without ;)
> I am still afraid of gecko end. How it handle this. Will it allow this. Do
> we need gecko changes ?
It's easy to check ;)
>
> [Discussion was very old, so i want to get update on it before starting :)]
Thanks !
Please ping me if you need anything more :)
Flags: needinfo?(felash)
Assignee | ||
Comment 73•9 years ago
|
||
I guess, we will need UI/UX for prompt box .
Thanks
Flags: needinfo?(felash)
Comment 74•9 years ago
|
||
(In reply to kumar rishav (:rishav_) from comment #73)
> I guess, we will need UI/UX for prompt box .
>
> Thanks
Redirect to UX Bryant.
Hi Bryant, could you please review the old decision again and see if there's any missing? Thanks!
Flags: needinfo?(felash) → needinfo?(bmao)
Comment 75•9 years ago
|
||
The prompt box is the same as any other we have in the system already...
Comment 76•9 years ago
|
||
(In reply to Steve Chung [:steveck] from comment #74)
> (In reply to kumar rishav (:rishav_) from comment #73)
> > I guess, we will need UI/UX for prompt box .
> >
> > Thanks
>
> Redirect to UX Bryant.
>
> Hi Bryant, could you please review the old decision again and see if there's
> any missing? Thanks!
Wow, Its a long discussion @@. I agree on the decision of allowing user to send empty message since it has its necessary function. Ayman's design suggestion is quite clear to me, and the prompt message is fine too.
Kumar, do you need anything from me for this implementation?
Flags: needinfo?(bmao) → needinfo?(rishav006)
Comment 78•9 years ago
|
||
Assignee | ||
Comment 79•9 years ago
|
||
Comment on attachment 8680467 [details]
[gaia] kumarrishav:Bug-935060 > mozilla-b2g:master
Hi Julien,
Here is the patch. Have a look. Hope it's ok. Yet to add comments though.
Thanks
Attachment #8680467 -
Flags: feedback?(felash)
Assignee | ||
Updated•9 years ago
|
Attachment #8680467 -
Flags: feedback?(felash) → review?(felash)
Comment 80•9 years ago
|
||
Comment on attachment 8680467 [details]
[gaia] kumarrishav:Bug-935060 > mozilla-b2g:master
Hey Oleg, do you have the bandwidth to review this ?
Thanks !
Attachment #8680467 -
Flags: review?(felash) → review?(azasypkin)
Comment 81•9 years ago
|
||
Comment on attachment 8680467 [details]
[gaia] kumarrishav:Bug-935060 > mozilla-b2g:master
Left several comments at github, but overall looks good + please rebase your PR on the latest master, it seems there are some integration tests that are failing right now with the PR.
Thanks!
Attachment #8680467 -
Flags: review?(azasypkin)
Updated•9 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•9 years ago
|
Attachment #8680467 -
Flags: review?(azasypkin)
Comment 82•9 years ago
|
||
Comment on attachment 8680467 [details]
[gaia] kumarrishav:Bug-935060 > mozilla-b2g:master
One recommendation at GitHub and please add one more unit test for the case when user doesn't confirm sending of an empty message. Also take a look at Treeherder - looks like unit tests are failing with your PR.
Please add your changes as a separate commit :)
Thanks!
Attachment #8680467 -
Flags: review?(azasypkin)
Comment 83•9 years ago
|
||
Hey Bryant,
Could you please confirm that an empty message looks acceptable from UX POV :)
Thanks!
Attachment #8692637 -
Flags: ui-review?(bmao)
Assignee | ||
Updated•9 years ago
|
Attachment #8680467 -
Flags: review?(azasypkin)
Comment 84•9 years ago
|
||
Sorry for the delay Rishav, just stumbled upon one more case where I'd like to have UX opinion.
Hey Bryant, here is question for you:
* Should we hide "Select text" context menu item for the empty messages? It's a bit confusing when you choose this menu item and nothing happens :)
P.S. "Forward" menu item for such messages basically opens empty New Message view, but maybe it's still better to have this option just for the consistency :)
Thanks!
Flags: needinfo?(bmao)
Comment 85•9 years ago
|
||
Comment on attachment 8680467 [details]
[gaia] kumarrishav:Bug-935060 > mozilla-b2g:master
Removing r? for now, let's wait for Bryant's replies first to have less review cycles :)
And the code looks good to me btw.
Attachment #8680467 -
Flags: review?(azasypkin)
Comment 86•9 years ago
|
||
(In reply to Oleg Zasypkin [:azasypkin][⏰UTC+1] from comment #84)
> Sorry for the delay Rishav, just stumbled upon one more case where I'd like
> to have UX opinion.
>
> Hey Bryant, here is question for you:
>
> * Should we hide "Select text" context menu item for the empty messages?
> It's a bit confusing when you choose this menu item and nothing happens :)
>
> P.S. "Forward" menu item for such messages basically opens empty New Message
> view, but maybe it's still better to have this option just for the
> consistency :)
>
> Thanks!
Hi Oleg,
Sorry for the late reply. Here are some of my suggestions:
1) Empty message looks perfect.
2) Do we have disable state for the menu item? if yes, i think it would be better to keep the select text in disable state.
3) Agree!
let me know if there are any problems ;)
Flags: needinfo?(bmao)
Assignee | ||
Comment 87•9 years ago
|
||
(In reply to Oleg Zasypkin [:azasypkin][⏰UTC+1] from comment #84)
>
> * Should we hide "Select text" context menu item for the empty messages?
> It's a bit confusing when you choose this menu item and nothing happens :)
>
IMO, it's fine to keep like this to maintain consistency. Also, if message is empty, then select text will also be empty, and it's obvious. Also, hardly anyone think about select text in case of message empty.
Thanks!
Sorry for delay .
Flags: needinfo?(azasypkin)
Comment 88•9 years ago
|
||
(In reply to kumar rishav (:rishav_) from comment #87)
> (In reply to Oleg Zasypkin [:azasypkin][⏰UTC+1] from comment #84)
> >
> > * Should we hide "Select text" context menu item for the empty messages?
> > It's a bit confusing when you choose this menu item and nothing happens :)
> >
> IMO, it's fine to keep like this to maintain consistency. Also, if message
> is empty, then select text will also be empty, and it's obvious. Also,
> hardly anyone think about select text in case of message empty.
My main point was that when user chooses "Select text" for the empty message nothing happens that may look like a bug, but we don't have support for disabled menu items right now, so let's keep it like this and improve later if needed.
Flags: needinfo?(azasypkin)
Assignee | ||
Comment 89•9 years ago
|
||
Comment on attachment 8680467 [details]
[gaia] kumarrishav:Bug-935060 > mozilla-b2g:master
Review? as It's OK from Bryant.
Attachment #8680467 -
Flags: review?(azasypkin)
Comment 90•9 years ago
|
||
Comment on attachment 8680467 [details]
[gaia] kumarrishav:Bug-935060 > mozilla-b2g:master
Sorry for the delay!
I've added some nits, but delaying r+ because of perma-red SMS notification and new_activity integration tests - please rebase your PR on the latest master (you can now squash all your commits into one if you prefer) and run integration tests.
Also keep in mind that new_activity and share_activity tests are disabled on Treeherder, but you can run them locally to make sure that they are green (local env should not be affected by intermittent issues). Sorry for such complexity :/
As soon as you're ready I'm at your disposal, without delay this time :)
Thanks a lot!
Attachment #8680467 -
Flags: review?(azasypkin)
Comment 91•9 years ago
|
||
Forgot to mention that for new_activity test you may want to wait for Steve's PR in bug 1235842 (it's about to land) - it fixes the reason why new activity tests are failing permanently.
Comment 92•9 years ago
|
||
Comment on attachment 8692637 [details]
empty-message.png
Setting ui-review+ (see Bryant's comment 86).
Attachment #8692637 -
Flags: ui-review?(bmao) → ui-review+
Assignee | ||
Updated•9 years ago
|
Attachment #8680467 -
Flags: review?(azasypkin)
Comment 93•9 years ago
|
||
Comment on attachment 8680467 [details]
[gaia] kumarrishav:Bug-935060 > mozilla-b2g:master
Sorry, but it looks like Gij 17 [1] is failing with your PR (notification tests in particular [2]) :/
Have you tried to run integration tests locally (to include temporarily disabled tests as well)?
[1] https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=e0de0f0eb3cfaee646d2b0219f6883c51751561f
[2] https://public-artifacts.taskcluster.net/K275-mnZTiCYPZqPi4Os1g/3/public/logs/live_backing.log
Attachment #8680467 -
Flags: review?(azasypkin)
Assignee | ||
Comment 94•9 years ago
|
||
Hi Oleg,
But it passed on my local branch. See the attached image.
Why this weird behavior ?
Thanks
Assignee | ||
Comment 95•9 years ago
|
||
(And now, you have my screenshot. :P. You can see my Username is Ultron and i am trying my hands on Visual studios ;) ha ha ha )
Flags: needinfo?(azasypkin)
Comment 96•9 years ago
|
||
(In reply to kumar rishav (:rishav_) from comment #95)
> Created attachment 8711370 [details]
> Screenshot from 2016-01-24 03-34-19.png
>
> (And now, you have my screenshot. :P. You can see my Username is Ultron and
> i am trying my hands on Visual studios ;) ha ha ha )
Nice! Could you please run new_activity and share_activity tests locally as well? And if they are green + green Treeherder - set r?! :)
Flags: needinfo?(azasypkin)
Comment 97•8 years ago
|
||
Mass closing of Gaia::SMS bugs. End of an era :(
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
Comment 98•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
•