Closed Bug 1030160 Opened 6 years ago Closed 6 years ago

[Messages][MMS] Subject is considered empty (placeholder is displayed) even if it has several empty lines

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect, minor)

ARM
Gonk (Firefox OS)
defect
Not set
minor

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.0 S5 (4july)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: azasypkin, Assigned: steveck)

References

Details

(Keywords: regression, Whiteboard: [not-part-of-initial-sprint])

Attachments

(2 files)

*** Follow-up from bug 1008127 ***

Subject is considered empty (placeholder is displayed) even if it has several empty lines that looks akward.

STR:

1. Open SMS app;
2. Go to the new message composer;
3. Go to the message composer options and select 'Add Subject';
4. Press "Enter";
5. Observe subject input.

Expected result:
"Subject" placeholder text should disappear (and converted to MMS?)

Actual result:
"Subject" placeholder text is still visible, message is still SMS.

It has been regressed by patch from bug 949457.
Since bug 949457 already landed in 2.0, nominate for 2.0 as well.
blocking-b2g: --- → 2.0?
can we remove the subject if it's empty line?
Flags: needinfo?(jelee)
Just to clarify:

1. either we remove the subject (incl any eomty line subject) from the messages and keep the item as a SMS
or
2. we keep the subject but then the message will become an MMS 

Call on UX for best user experience.
UX,

Please take a look.
Flags: needinfo?(firefoxos-ux-bugzilla)
Flagging the correct Comms UX people here.
Flags: needinfo?(vpg)
Flags: needinfo?(jelee)
Flags: needinfo?(firefoxos-ux-bugzilla)
Flags: needinfo?(cawang)
I think Jenny handles Message app UX now.
Although she is on PTO, but should be back next week to reply this.
Flags: needinfo?(jelee)
blocking-b2g: 2.0? → 2.0+
Hello all, I prefer this solution: Right after adding subject (nothing typed yet), pressing enter shouldn't do anything, i.e cursor remains at the same position and placeholder text stays too. Once user starts typing, placeholder text disappears and user can change line by pressing "enter". This is the same as recipient field current behavior, when there's no contact/number input yet, pressing "enter" will do nothing. Tks!
Flags: needinfo?(jelee)
Flags: needinfo?(cawang)
(In reply to Jenny Lee from comment #7)
> Hello all, I prefer this solution: Right after adding subject (nothing typed
> yet), pressing enter shouldn't do anything, i.e cursor remains at the same
> position and placeholder text stays too. Once user starts typing,
> placeholder text disappears and user can change line by pressing "enter".
> This is the same as recipient field current behavior, when there's no
> contact/number input yet, pressing "enter" will do nothing. Tks!

Hey Jenny,

There's one more case when we still can have only empty lines:

* Type a single char into newly added subject;
* Press enter (now we have two lines, one with char and second is empty);
* Remove char from the first line with backspace(now we have two empty lines).

What should we do in this case?

Thanks!
Flags: needinfo?(jelee)
(In reply to Oleg Zasypkin [:azasypkin] from comment #8)
> (In reply to Jenny Lee from comment #7)
> > Hello all, I prefer this solution: Right after adding subject (nothing typed
> > yet), pressing enter shouldn't do anything, i.e cursor remains at the same
> > position and placeholder text stays too. Once user starts typing,
> > placeholder text disappears and user can change line by pressing "enter".
> > This is the same as recipient field current behavior, when there's no
> > contact/number input yet, pressing "enter" will do nothing. Tks!
> 
> Hey Jenny,
> 
> There's one more case when we still can have only empty lines:
> 
> * Type a single char into newly added subject;
> * Press enter (now we have two lines, one with char and second is empty);
> * Remove char from the first line with backspace(now we have two empty
> lines).
> 
> What should we do in this case?
> 
> Thanks!

Maybe we can ignore all the new line input(or focus to message input when enter pressed)?
Assignee: nobody → schung
Flags: needinfo?(vpg)
(In reply to Oleg Zasypkin [:azasypkin] from comment #8)

> Hey Jenny,
> 
> There's one more case when we still can have only empty lines:
> 
> * Type a single char into newly added subject;
> * Press enter (now we have two lines, one with char and second is empty);
> * Remove char from the first line with backspace(now we have two empty
> lines).

Hello Oleg! I can't seem to reproduce the steps, what I'm seeing is that when backspace to clear the first line, placeholder text reappears and there's no second line.
Assignee: schung → nobody
Flags: needinfo?(jelee)
I'd like that we focus on fixing the regression over 1.4 rather than doing new stuff. Let's file a separate bug for this.
Assignee: nobody → schung
(In reply to Jenny Lee from comment #10)

> Hello Oleg! I can't seem to reproduce the steps, what I'm seeing is that
> when backspace to clear the first line, placeholder text reappears and
> there's no second line.

Steve showed me how it's reproduced. Thank you Steve!
To prevent the inconsistency between subject input field (multiple empty lines) and actual display (no empty line), let's ignore all the new line input in subject input field. Thanks!
(In reply to Julien Wajsberg [:julienw] (away June 21 to 30) from comment #11)
> I'd like that we focus on fixing the regression over 1.4 rather than doing
> new stuff. Let's file a separate bug for this.

The behavior in v1.4 is we also treat new line char as a valid char, so pressing enter will convert to mms. Message with new line char subject is still able to send, but sender and receiver could only see the empty subject mms.
Attached file Link to github
Hi Julien, since Jenny suggested that we should not revert to the behavior in v1.4 per discussion with her today, I made a small PR that just ignore the retun key while in subject. Return key still got some functionality that could choose the first matched suggestion string, so disabling the input seems not that awkward at all :p
Attachment #8449199 - Flags: review?(felash)
Blocks: sms-sprint-4
Whiteboard: [not-part-of-initial-sprint]
Comment on attachment 8449199 [details] [review]
Link to github

Feedback? to Oleg for possible remaining work-around in bug 1008127
Attachment #8449199 - Flags: feedback?(azasypkin)
Comment on attachment 8449199 [details] [review]
Link to github

(In reply to Steve Chung [:steveck] from comment #15)
> Comment on attachment 8449199 [details] [review]
> Link to github
> 
> Feedback? to Oleg for possible remaining work-around in bug 1008127

Hey Steve, you're right, the latest patch for bug 1008127 removed workaround for sms message with multiline empty subject, so you don't need to do anything with it. Thanks!
Attachment #8449199 - Flags: feedback?(azasypkin)
Comment on attachment 8449199 [details] [review]
Link to github

I played with this a bit, and it feels strange that we don't do anything when we press "enter". Especially, when a keyboard suggestion is automatically entered, we don't have a space at the end, and it feels weird. So I'd suggest converting the "enter" to a "space" instead of merely ignoring it. NI Jenny about this.

I don't know how it will fit with the keyboard, maybe we need to dispatch a KeyboardEvent manually and not simply appending to the message...
Flags: needinfo?(jelee)
I'm ok with pressing "enter" in subject line means "choose suggestion + space", if there's no word suggestion to choose from, pressing "enter" still does nothing. Tks!
Flags: needinfo?(jelee)
(In reply to Jenny Lee from comment #18)
> I'm ok with pressing "enter" in subject line means "choose suggestion +
> space", if there's no word suggestion to choose from, pressing "enter" still
> does nothing. Tks!

Sorry we could not make this requirement because there is no way to know if the string comes from suggestion or not...:-/

And dispatch KeyboardEvent to subject seems not working either(I tried to dispatch a keydown event to subjuct but content didn't change). Do we still need to append the space to the end? I'm afraid that insert the inserting char manully might cause other unexpected bahavior between keyboard.
Flags: needinfo?(felash)
(In reply to Steve Chung [:steveck] from comment #19)
> (In reply to Jenny Lee from comment #18)
> > I'm ok with pressing "enter" in subject line means "choose suggestion +
> > space", if there's no word suggestion to choose from, pressing "enter" still
> > does nothing. Tks!
> 
> Sorry we could not make this requirement because there is no way to know if
> the string comes from suggestion or not...:-/
> 
> And dispatch KeyboardEvent to subject seems not working either(I tried to
> dispatch a keydown event to subjuct but content didn't change). Do we still
> need to append the space to the end? I'm afraid that insert the inserting
> char manully might cause other unexpected bahavior between keyboard.

Can you try this ? Should be quick to check !
Flags: needinfo?(felash)
Attached patch test.patchSplinter Review
Hi Julien, I made a patch will manual space input, but it didn't work as I expect: The return could only add one space at the end, but it will still exceed the limitation if you press return continuously even the following space didn't add. And another issue is the cursor didn't display after space added. Do you any feedback about the manual space insert function? Thanks.
Attachment #8450868 - Flags: feedback?(felash)
Comment on attachment 8449199 [details] [review]
Link to github

I addressed the suggestion, and I also create another testing patch per your request. But since Jenny didn't prefer that we append space for every cases, you may need to discuss with her about the space insertion behavior, thanks!
About the cursor: I think there is a platform bug, can you try to do a simple page exposing the issue, and file another bug?

I found other strange issues with this:
* press enter
* add one character
* press backspace
=> this doesn't seem to remove something and the placeholder does not come back

I tried other things too but nothing that works as I'd like too, so let's move forward with your first patch instead.
Attachment #8450868 - Flags: feedback?(felash)
Comment on attachment 8449199 [details] [review]
Link to github

r=me

let's move forward, thanks !
Attachment #8449199 - Flags: review?(felash) → review+
master: 312dd839ea19cfb28bd95e6c8923115cc062ced2
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.