1164857 - App reviews can be sent with only blank spaces

Status: RESOLVED WONTFIX

(Marketplace Graveyard :: Consumer Pages, defect, P3)

Description

[Victor Carciu]

Prerequisites:
Build identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:41.0) Gecko/20100101 Firefox/4

Steps to reproduce:
1.Go to any free app details page and write a review containing only blank spaces

Expected results:
Users should not be able to send reviews with only blank spaces

Actual results:
Reviews with only blank spaces are successfully submitted 

Please see screencast for this bug: http://screencast.com/t/qM7kQkI9G

Comment 1

[Ram Dayal Vaishnav [:ram]]

Could you please give some directions to the volunteer to fix this ?

Comment 2

[Bharath Thiruveedula[:tbharath]]

Can I work on this?

Comment 3

[David Durst [:ddurst]]

Please. Reviews should not be submittable if there has been no text added, or if the trimmed text equals an empty string.

Comment 4

[Sukhwinder Singh Sehgal]

https://github.com/mozilla/fireplace/pull/1231

Comment 5

[Ram Dayal Vaishnav [:ram]]

Previous PR is closed, created new one - https://github.com/mozilla/fireplace/pull/1256/files. Please review.

Comment 6

[Victor Carciu]

I think that the Submit Review button should be inactive if empty string is added. Now we are returning an internal server error when an empty string is submitted, which is confusing.

Comment 7

[Sukhwinder Singh Sehgal]

I am working on this bug.

Comment 8

[Valentina Virlics [:vvalentina], Ecosystem QA]

Now, on MP-dev FF41(Win 7), reviews with special characters only(@, $, %...) cannot be submitted. Don't know if is a regression from this issue or an intended behavior.
Please see the screencast: http://screencast.com/t/TSjIGa1Z

Comment 9

[David Durst [:ddurst]]

Meaning: reviews that consist of *only* special characters, or reviews that have at least one special character in them?

Comment 10

[Victor Carciu]

(In reply to David Durst [:ddurst] from comment #9)
> Meaning: reviews that consist of *only* special characters, or reviews that
> have at least one special character in them?

Reviews that consist of *only* special characters

Comment 11

[David Durst [:ddurst]]

I don't know if that's a regression or intended either, but I think it should be intended.

Comment 12

[Valentina Virlics [:vvalentina], Ecosystem QA]

(In reply to David Durst [:ddurst] from comment #11)
> I don't know if that's a regression or intended either, but I think it
> should be intended.

Maybe someone wants to underline as quickly as he can that the app is expensive and write like a review "$$$!"; I know is very unlikely, but it can happen...
If is intended, a message in the review popup window will be useful. Just an opinion. Thanks!

Comment 13

[David Durst [:ddurst]]

I'm happy to consider this a nudge towards a world that must use words.

Comment 14

[Victor Carciu]

(In reply to David Durst [:ddurst] from comment #13)
> I'm happy to consider this a nudge towards a world that must use words.

This bug is still reopened based on comment #6. 
We should activate the Submit button only when valid text is entered.

Comment 15

[David Durst [:ddurst]]

Is that the case? I was under the impression that that was resolved as of comment #7.

Comment 16

[Victor Carciu]

(In reply to David Durst [:ddurst] from comment #15)
> Is that the case? I was under the impression that that was resolved as of
> comment #7.

No, this was not resolved

Comment 17

[David Durst [:ddurst]]

I'm not sure I understand. I cannot reproduce the originally described issue on -dev.

This would appear to be resolved fixed.

Comment 18

[David Durst [:ddurst]]

Oh, nevermind, user error.

Comment 19

[Trishul]

Attachment: Screenshot from 2015-07-01 08:37:42.png

I think this is still not fixed. Tested on production, accepts empty string and dismisses the modal. After dismiss it gives error message.

Comment 20

[Ram Dayal Vaishnav [:ram]]

Any hint on how to keep the Submit Review button inactive until the review string is invalid ?

Comment 21

[Amy Tsay [:amyt]]

According to ddurst there are two things: 1) keeping the button disabled until 2) the string is valid. I assume this isn't something that requires special knowledge from a Marketplace staff dev, but can be figured out by tinkering with the code?

Comment 22

[Elizabeth Hunt[:ezoehunt]]

Update information on how Submit a Review should work --

The only requirement is that the user select a star rating. So a user can submit a review with no text as long as they've selected a star rating. Please update Submit a Review so that as soon as a user selects a star rating, the Submit button becomes active (blue).

Also :ddurst says that we do want to make sure any non-whitespace text that is submitted is not just junk. So disallow text like "^$%%#&#@*$@@" but allow whitespace (empty input field).

Comment 23

[Ram Dayal Vaishnav [:ram]]

Can someone be assigned here as Mentor ? I will need some help in code changes to fix this. I think I will have to go beyond the html & js file to make is possible to submit Review without review text. Please guide me on this.

html - https://github.com/mozilla/fireplace/blob/master/src/templates/ratings/add.html
js - https://github.com/mozilla/fireplace/blob/master/src/media/js/reviews.js

Thanks,
Ram

Comment 24

[madhukarkrazzyrocker]

Can I work on this project ?

Comment 25

[Ram Dayal Vaishnav [:ram]]

Hi Madhu,

It looks like you will have to wait till someone from team answer comment#23. I can suggest you to work on https://bugzilla.mozilla.org/show_bug.cgi?id=1184424, as this is a good 'ready to pick' good first bug.

NI'ing Kevin for comment#23.

Thanks,
Ram

Comment 26

[Ram Dayal Vaishnav [:ram]]

Hi David, can you please opine on comment 23?

Comment 27

[David Durst [:ddurst]]

Before the submission can be allowed, three things have to be true:
1. star rating has been selected
2. any entered text has been stripped of leading and trailing whitespace
3. the remaining text must not be just punctuation characters

Adding ngoke as mentor, since he nom'd this originally.

Comment 28

[Ram Dayal Vaishnav [:ram]]

Setting assignee to none as I am working on some other bug, leaving it for newbies. Anyone who want to work on this can comment here.

Comment 29

[Mohammad Shahbaz Alam [:shahbaz17]]

I want to work on this bug.

Comment 30

[Ram Dayal Vaishnav [:ram]]

Thanks Shahbaz, assigning you to this bug.

Comment 31

[David Durst [:ddurst]]

(In reply to David Durst [:ddurst] from comment #27)
> Before the submission can be allowed, three things have to be true:
> 1. star rating has been selected
> 2. any entered text has been stripped of leading and trailing whitespace
> 3. the remaining text must not be just punctuation characters
> 
> Adding ngoke as mentor, since he nom'd this originally.

Ignore my #3 -- for the purposes of this bug, all non-whitespace text is valid.

So these two things have to be true:
1. star rating has been selected
2. any entered text has been stripped of leading and trailing whitespace

Comment 32

[Ram Dayal Vaishnav [:ram]]

Hi Shahbaz, any progress on this? Let us know if you need any help.

Comment 33

[David Durst [:ddurst]]

Any progress on this? If this is still unchanged in two weeks, we'll unassign it.

Comment 34

[Atique Ahmed Ziad [:atiqueahmedziad]]

Unassigned due to inactivity.

Comment 35

[Vince Tieu [:vtieu]]

Attachment: First attempt

This is my first attempt at fixing the bug. The form cannot be submitted if the user has only whitespace, however the submit button is still green.

Comment 36

[David Durst [:ddurst]]

We had talked about this being solved by the same thing as bug 1164864. That has a PR (https://bugzilla.mozilla.org/show_bug.cgi?id=1164864#c18) -- so should we go with that or do we need to also look at this patch?

Comment 37

[Vince Tieu [:vtieu]]

(In reply to David Durst [:ddurst] from comment #36)
> We had talked about this being solved by the same thing as bug 1164864. That
> has a PR (https://bugzilla.mozilla.org/show_bug.cgi?id=1164864#c18) -- so
> should we go with that or do we need to also look at this patch?

Yup. Both bugs seem to be fixed now.

Comment 38

[Atique Ahmed Ziad [:atiqueahmedziad]]

Before closing the bug, 

I'm getting a console error when trying to give white-spaces in textarea, it's gives - ReferenceError: $currentForm is not defined

@Vince, typo here- https://github.com/mozilla/marketplace-core-modules/pull/53/files#diff-bc00cccc6504fefcfb8646bb70338da7R8

Another issue is, in comment 22 --ezoehunt had said that selecting stars ratings only are enough for submitting review. @ddurst: do we need this?

Comment 39

[Atique Ahmed Ziad [:atiqueahmedziad]]

Again in comment 6 it was suggested that the submit button should remain inactive if there is empty string in textarea. 
But it seems that the above patch is working a bit differently. Here, the submit button becomes active as soon as you give white spaces & if then we click on submit button, it shows this msg : "There was an issue submitting your report. Please try again later."

Here, the wording for not accepting blank report may need to change or may the submit button be inactive if there is only white spaces.

ddurst: Which one is preferable?

Comment 40

[Atique Ahmed Ziad [:atiqueahmedziad]]

Ignore my comment 39 , maybe I got the unexpected behavior due to wrong variable name. Clearing the NI for Vince as spasovski fixed the variable name.

Comment 41

[David Durst [:ddurst]]

(In reply to Atique Ahmed Ziad [:atiqueahmedziad] from comment #38)
> Another issue is, in comment 22 --ezoehunt had said that selecting stars
> ratings only are enough for submitting review. @ddurst: do we need this?

Yes.

Comment 42

[Atique Ahmed Ziad [:atiqueahmedziad]]

 (In reply to David Durst [:ddurst] from comment #41)
> (In reply to Atique Ahmed Ziad [:atiqueahmedziad] from comment #38)
> > Another issue is, in comment 22 --ezoehunt had said that selecting stars
> > ratings only are enough for submitting review. @ddurst: do we need this?
> 
> Yes.

Thanks ddurst for the feedback. 

Vince, you need to do a little more work to close this bug. As it is said, users can submit the review form by selecting star rating only. Can you make a patch for it too?

Comment 43

[Vince Tieu [:vtieu]]

Will get to work on this. Thanks everyone.

Comment 44

[BMO Automation]

Firefox Marketplace has been shut down since March. Closing remaining bugs.