1342906 - [Control Center] Validate fields against expected values

Status: RESOLVED WONTFIX

(Firefox :: Normandy Server, defect)

Description

[Adrian Florinescu [:aflorinescu]]

[Description:]
It is expected that human error might happen when pasting or manually imputing values for a certain recipe in the Shield Control Center. Validation for each of these fields is expected.

[Prerequisites]:
Open FF/ about:config and
1. Set the extensions.shield-recipe-client.dev_mode preference to true to run recipes immediately on startup.
2. Set the extensions.shield-recipe-client.logging.level preference to 0 to enable more logging.
3. Set the security.content.signature.root_hash preference to DB:74:CE:58:E4:F9:D0:9E:E0:42:36:BE:6C:C5:C4:F6:6A:E7:74:7D:C0:21:42:7A:03:BC:2F:57:0C:8B:9B:90.


[Steps:]
1. Open Control Center.
2. From the right corner press Add New button.
3. From Action, select Heartbeat Prompt.
4. Message Field.
5. Engagement Button Label.
6. Thanks message.
7. Post-answer URL.
8. Learn More Message.
9. Learn More URL.
10. From the field "how Often should the prompt be shown", select the 3rd option: "allow re-prompting users who have already seen this prompt after x days since they last saw it".

[Expected Result:]
4. Message field should be limited to x chars, due to bug 1342094 and bug 1342399. One solution here would be to also wrap the message, but I'm not sure if this is really a path we want to asses as an option !?.
5. Since it's a button, trim left and limit the length to 20-25 chars + spaces.
6+8. Since these are messages, their lenght should be limited as well, as well as left trimmed.
7+9. Given the fact that it is an url, it should be validated as such, otherwise show user an error message. Also since Mozilla is going to be releasing the Insecure Contextual Warning for user/password (bug 1217152) we might validate it to be a https, although not sure if this limitation should be added.
10. A new textbox will be shown, requiring days to be input before the user is re-prompted: this field is expected to validate positive and maybe integer? values. (I believe here would be useful for testing purposes to have float values as well) - also might be reasons to have values like 1.5 days

Comment 1

[Adrian Florinescu [:aflorinescu]]

Mike, let me know if you'd prefer to split each or some of the above into separate bugs to be easier to track. When I've created this bug, I've thought that the above are straight forward items and since the action items are to be handled in github, it's going to be easier to track them all.

Comment 2

[Osmose [:osmose, :mkelly]]

(In reply to Adrian Florinescu [:AdrianSV] from comment #1)
> Mike, let me know if you'd prefer to split each or some of the above into
> separate bugs to be easier to track. When I've created this bug, I've
> thought that the above are straight forward items and since the action items
> are to be handled in github, it's going to be easier to track them all.

Sounds good to me.

(In reply to Adrian Florinescu [:AdrianSV] from comment #0)
> [Description:]
> 4. Message field should be limited to x chars, due to bug 1342094 and bug
> 1342399. One solution here would be to also wrap the message, but I'm not
> sure if this is really a path we want to asses as an option !?.

This might be a good stopgap if we can't find a better fix for those bugs. Wrapping the text is something we had planned for future Heartbeat improvements (see the issue: https://github.com/mozilla/normandy/issues/331, and the mockups: https://mozilla.invisionapp.com/share/DN9J815MY#/screens/208582210).

I filed https://github.com/mozilla/normandy/issues/563 for bug 1342094, which covers this, I think.

> 5. Since it's a button, trim left and limit the length to 20-25 chars +
> spaces.
> 6+8. Since these are messages, their lenght should be limited as well, as
> well as left trimmed.

https://github.com/mozilla/normandy/issues/563 covers all of them, as mitigation will be similar for each of them.

> 7+9. Given the fact that it is an url, it should be validated as such,
> otherwise show user an error message. Also since Mozilla is going to be
> releasing the Insecure Contextual Warning for user/password (bug 1217152) we
> might validate it to be a https, although not sure if this limitation should
> be added.

Heartbeat prompts don't generally lead to a page with login credentials, so I don't think we need to validate HTTPS. Validating the URL is filed under https://github.com/mozilla/normandy/issues/533.

> 10. A new textbox will be shown, requiring days to be input before the user
> is re-prompted: this field is expected to validate positive and maybe
> integer? values. (I believe here would be useful for testing purposes to
> have float values as well) - also might be reasons to have values like 1.5
> days

For technical reasons, a float value isn't ideal, but changing the duration from days to minutes is a good idea IMO. We've got https://github.com/mozilla/normandy/issues/533 on file for that. We've also got https://github.com/mozilla/normandy/issues/537 for validating that it's a positive number. The number input itself should already be validating that it's an integer; I wasn't able to input a float last time I checked, but I might be wrong.

Comment 3

[Michael Cooper [:mythmon]]

I think everything here was handled or is a wontfix.