After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 779621 - Restrict the input for "preparation time" and "cooking time"
Restrict the input for "preparation time" and "cooking time"
Status: RESOLVED FIXED
Product: recipes
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Recipes maintainer(s)
Recipes maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2017-03-05 16:57 UTC by Ekta Nandwani
Modified: 2017-03-28 20:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Both the fields accepting any random string (55.72 KB, image/png)
2017-03-05 16:57 UTC, Ekta Nandwani
  Details
30 characters (239.75 KB, image/png)
2017-03-07 03:12 UTC, Matthias Clasen
  Details
Limit preptime_combo and cook_time_combo text to 30 chars (1.71 KB, patch)
2017-03-17 12:35 UTC, Ekta Nandwani
none Details | Review
Restricts all the entries on edit-page to 30 chars (4.16 KB, patch)
2017-03-24 06:51 UTC, Ekta Nandwani
none Details | Review
Restricts all the entries on edit-page to 30 chars (5.54 KB, patch)
2017-03-28 19:54 UTC, Ekta Nandwani
none Details | Review

Description Ekta Nandwani 2017-03-05 16:57:09 UTC
Created attachment 347271 [details]
Both the fields accepting any random string

Both the fields accept any random strings.
Comment 1 Matthias Clasen 2017-03-06 01:28:29 UTC
My take on this is that we should do enough validation to prevent issues like crashes, or inability to load the database the next time. Things we can consider:

- disallow newlines
- limit the length of the input
Comment 2 Ekta Nandwani 2017-03-06 04:42:32 UTC
(In reply to Matthias Clasen from comment #1)
> My take on this is that we should do enough validation to prevent issues
> like crashes, or inability to load the database the next time. Things we can
> consider:
> 
> - disallow newlines
> - limit the length of the input

Ideally a user should not put in a string like that but disallowing newlines and limiting input length still wont stop users to type "abc" as the preparation time and cooking time.
 
Is removal the editable feature of combobox too much of a restriction? Because it still gives a broad idea of around how long does the recipe takes.
Comment 3 Matthias Clasen 2017-03-06 16:48:25 UTC
no, that may be just fine. Lets see if I can Jakub to chime in here.
Comment 4 Jakub Steiner 2017-03-06 17:42:53 UTC
Whatever you come up with to restrict the format, I favor trying to convert the format to alerting a format being incorrect. I don't consider allowing "abc" for time as much of a deal to disallowing "Roughly 2 hours". 

The only somewhat real use case for being strict with preparation time format is search. But perhaps searching for recipes that will "take less than 20 minutes to make, because I need to leave for a plane in an hour" is a bit of a stretch.
Comment 5 Matthias Clasen 2017-03-06 19:15:26 UTC
I guess that makes sense - we don't need this field to be machine-readable, so any restrictions we apply should be targeted at making sure that we get a string that doesn't break the database and doesn't break the layout of the details page or the print output. A value like 'abc' might not make for a useful preparation time, but it doesn't do any actual harm...
Comment 6 Ekta Nandwani 2017-03-06 20:51:30 UTC
(In reply to Matthias Clasen from comment #5)
> I guess that makes sense - we don't need this field to be machine-readable,
> so any restrictions we apply should be targeted at making sure that we get a
> string that doesn't break the database and doesn't break the layout of the
> details page or the print output. A value like 'abc' might not make for a
> useful preparation time, but it doesn't do any actual harm...

Hence,
- disallow newlines
- limit the length of the input. What would be the limit of length?
Comment 7 Matthias Clasen 2017-03-07 03:11:54 UTC
as much as can fit on the details page, looks like 30 characters is the answer...
Comment 8 Matthias Clasen 2017-03-07 03:12:12 UTC
Created attachment 347359 [details]
30 characters
Comment 9 Ekta Nandwani 2017-03-17 12:35:45 UTC
Created attachment 348175 [details] [review]
Limit preptime_combo and cook_time_combo text to 30 chars
Comment 10 Matthias Clasen 2017-03-19 23:13:13 UTC
Review of attachment 348175 [details] [review]:

Works fine in my testing. Lets go with it.
Comment 11 Matthias Clasen 2017-03-19 23:14:05 UTC
We should put this patch on both the master and the recipes-1.0 branch.
Comment 12 Matthias Clasen 2017-03-20 08:13:00 UTC
and we should probably do the same for the other entries on the edit page
Comment 13 Ekta Nandwani 2017-03-24 06:51:30 UTC
Created attachment 348633 [details] [review]
Restricts all the entries on edit-page to 30 chars
Comment 14 Matthias Clasen 2017-03-25 00:02:44 UTC
Review of attachment 348633 [details] [review]:

Patch itself looks great; can you put something in the commit message body when
you push this ?

Like

Allowing arbitrarily long strings for these fields is not necessary and breaks
the layout of the details page.
Comment 15 Ekta Nandwani 2017-03-28 19:54:02 UTC
Created attachment 348899 [details] [review]
Restricts all the entries on edit-page to 30 chars