Page MenuHomeWildfire Games

Support custom buttons in the termsdialog and use it for mod.io legal links and UserReporter config paths and publications link
ClosedPublic

Authored by elexis on Sep 15 2018, 1:02 PM.

Details

Summary

This allows pages that use the terms and conditions UI to define custom buttons that allow the user to visit a link or examine files prior to accepting the terms and conditions.

Implementation of legal requirements:

  • mod.io Terms and Conditions seem to be a contract that every user agrees to by using the service. So the user should be informed thereof or the contract might be void.
  • GDPR requires the mod.io users to be informed of various rights that should be present in the Privacy Policy of mod.io - at the time the service is used IIRC.

Arguably implementation of legal requirements:

  • transparency: UserReporter user can see the data he is goign to upload before it is uploaded following rP21868 and this information dialog.
  • transparency: UserReporter use can see how his data is going to be used on the website.
  • right to access: user can check the UserReporterID and might ideally use it as an authentication token to exercise GDPR rights without necessarily having to identify the natural person

Also:

  • Introduces strings.
  • copy&paste is still broken on unix. sometimes paste, sometimes no pasting, sometimes a crash.
  • code nice because rP21886, rP21866 and rP21887
  • passes on DMCA legal option to the user (dunno if it's good practice or a requirement).
Test Plan
  • needs decision whether the strings are the best to introduce and sufficiently worth the translators time
  • needs decision whether the UI and styling is the best and most suitable UI we can come up with and implement
  • needs confirmation that the JS mods are sufficiently untrustworthy that we send the user to locate config and log files given by the paths, rather than displaying all of his hardware data and the UserReporterID in JS.
  • needs decision whether the UserReport JSInterface config functions are as nice as they could be
  • needs installation on the WFG UserReporter backend: mirror Philips UserReporter publication at http://feedback.wildfiregames.com/. It's a 3MB 7z file, 32mb .tar.gz., 500mb unzipped and only needs to be unzipped there.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Sep 15 2018, 1:02 PM
elexis updated the Trac tickets for this revision.Sep 15 2018, 1:02 PM
Vulcan added a subscriber: Vulcan.Sep 15 2018, 1:09 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/mod/gui/msgbox/msgbox.js
|  54| »   switch·(captions.length)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.
|    | [NORMAL] ESLintBear (no-extra-semi):
|    | Unnecessary semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/common/functions_msgbox.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/common/functions_msgbox.js
|  24|  24| 
|  25|  25| 	g_MessageBoxBtnFunctions = [];
|  26|  26| 	g_MessageBoxCallbackArgs = [];
|  27|    |-};
|    |  27|+}
|  28|  28| 
|  29|  29| function messageBox(mbWidth, mbHeight, mbMessage, mbTitle, mbButtonCaptions, mbBtnCode, mbCallbackArgs, mbSelectable)
|  30|  30| {

binaries/data/mods/mod/gui/common/functions_msgbox.js
|  27| };
|    | [NORMAL] JSHintBear:
|    | Unnecessary semicolon.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/termsdialog/termsdialog.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/termsdialog/termsdialog.js
|   8|   8| 
|   9|   9| 	Engine.GetGUIObjectByName("mainText").caption =
|  10|  10| 		Engine.FileExists(data.file) ?
|  11|    |-		Engine.TranslateLines(Engine.ReadFile(data.file)) :
|    |  11|+			Engine.TranslateLines(Engine.ReadFile(data.file)) :
|  12|  12| 		data.file;
|  13|  13| 
|  14|  14| 	initCustomButtons(data.buttons);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/termsdialog/termsdialog.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/termsdialog/termsdialog.js
|   9|   9| 	Engine.GetGUIObjectByName("mainText").caption =
|  10|  10| 		Engine.FileExists(data.file) ?
|  11|  11| 		Engine.TranslateLines(Engine.ReadFile(data.file)) :
|  12|    |-		data.file;
|    |  12|+			data.file;
|  13|  13| 
|  14|  14| 	initCustomButtons(data.buttons);
|  15|  15| }

Link to build: https://jenkins.wildfiregames.com/job/differential/734/display/redirect

smiley added a comment.EditedSep 15 2018, 4:23 PM

What about button styles? Not necessary to allow it to be changeable, but the buttons should be styled according to the theme (at least an outline). Currently looks kinda out of place and ugly. Could go on a separate diff too. Another convenience would be to actually display the things instead of a path. But not high priority for now.

In D1627#64832, @smiley wrote:

What about button styles? Currently looks kinda out of place and ugly.

See second point of the test plan. Red buttons appear even uglier to me. I wanted it to become blue hyperlinks like we know from browsers, but there is no font for underlining and blue looks ugly. If you have a better style, share it.

Another convenience would be to actually display the things instead of a path

See third point of the test plan.

Maybe it would look better if the text is highlighted on mouse hover. Links could turn blue from white. I experimented with the following, maybe I missed an .onHover event?

// fix colors ofc.
button.onMouseEnter = () => {
	button.caption = coloredText(buttonData.caption, "red");
};			
button.onMouseLeave = () => {
	button.caption = buttonData.caption;
};
In D1627#64834, @smiley wrote:

Maybe it would look better if the text is highlighted on mouse hover. Links could turn blue from white. I experimented with the following, maybe I missed an .onHover event?

// fix colors ofc.
button.onMouseEnter = () => {
	button.caption = coloredText(buttonData.caption, "red");
};			
button.onMouseLeave = () => {
	button.caption = buttonData.caption;
};

Would have said GUIM_MOUSE_ENTER and GUIM_MOUSE_LEAVE (GUIbase.h) which trigger mouseleave, mouseenter events (CGUI.cpp).
But it's quite ugly to write code that changes the font depending on the hoverstate.
We even have textcolor_over for buttons (CButton.cpp).

Some monolog:

needs decision whether the strings are the best to introduce and sufficiently worth the translators time

  1. Unsure about the "Publications" button opening http://feedback.wildfiregames.com/.

Vladislav mentioned that he doesn't like the label and proposed "Statistics", which however doesn't convince me.
Using the URL as the button caption doesn't transport the use case for clicking on the button however, while "Publications" might do a bit more. The URL is still visible in the tooltip and the opening messagebox.

  1. Moving translate("Opening %(url)s\n in default web browser. Please wait…") from public/ to mod/ is tragic, but given that there will be many new strings, seems there is no reason to move it, given that it enlightens the user that he should look at the webbrowser now for a new tab (as is done in the other places in public/).
  1. button tooltip "translate("Open %(url)s in the browser."), I hope this is cheap enough. Reusing the first sentence of the string in 2. doesn't work due to the grammar, so that's the easiest string to translate if there will be one.

needs confirmation that the JS mods are sufficiently untrustworthy that we send the user to locate config and log files given by the paths, rather than displaying all of his hardware data and the UserReporterID in JS.

are

needs decision whether the UserReport JSInterface config functions are as nice as they could be

are

needs installation on the WFG UserReporter backend: mirror Philips UserReporter publication at http://feedback.wildfiregames.com/. It's a 3MB 7z file, 32mb .tar.gz., 500mb unzipped and only needs to be unzipped there.

In theory we can commit a dead link too and implement the dead link later.
Rephrasing the link button to not speak of the content of that link fails to achieve the point of being much more transparent with a single hyperlink.
Given that feedback.wildfiregames.com still refers to Philips server that has the reports, it's currently an active link and one may chose to only update the DNS config once the unzipping of the archive was successful.

needs decision whether the UI and styling is the best and most suitable UI we can come up with and implement

This was my biggest concern.

The feature appears like this with a texthover effect. Not too ugly, but it is still not obvious that these are buttons.

Using the regular button theme is confusing to the user, since it appears like there are 5 different choices how to respond to this popup.

It also becomes apparent that the indirection to open a button to see the config path is only adding confusion.
So annulling the userreporter logPath and configPath buttons.
Instead the two paths can be printed in the terms document itself using casual sprintf:

The current logfolder is [font="sans-bold-12"]%(logPath)s[/font]. The current configuration folder is [font="sans-bold-12"]%(configPath)s[/font].

  • This has the disadvantage that it's not as obvious to the user anymore that he can see the data before sending it (without allowing malicious JS mods to leak it).
  • This has the advantage that the user is provided with sufficient context.
  • It also has the disadvantage that one cannot select the folder anymore, because multiline input elements don't support gui tags and the patch to do that is too elaborative. The log and config paths are however short enough to type or locate manually.
  • The third disadvantage of this layout is that only 2 buttons can fit beside the Accept and Decline button, which means we have to drop the DMCA / copyright button from mod.io...

So the simple problem is solveable in any webbrowser with html but not with our custom source/gui/ from 2004 and
we can only chose in which way we will be unsatisfied by the patch.
Arbitrarily deciding for the last following all the feedback.

The sprintf part of the diff is implemented on git and will not be comitted to svn until the string translate("...%(logPath)s...") will be committed along the Terms and Conditions of the UserReporter.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 22 2018, 7:01 PM
This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Sep 22 2018, 7:01 PM

Good job. 👍
Is it intended, that the Enable Feedback button is first greyed out and the user has to click on technical details and then on connect to enable the user report?

In D1627#65047, @Imarok wrote:

Good job. 👍
Is it intended, that the Enable Feedback button is first greyed out and the user has to click on technical details and then on connect to enable the user report?

Pretty sure it's intended. They are supposed to read and accept the terms before doing it I guess.

elexis added a comment.EditedSep 24 2018, 1:08 PM
In D1627#65048, @smiley wrote:
In D1627#65047, @Imarok wrote:

Good job. 👍
Is it intended, that the Enable Feedback button is first greyed out and the user has to click on technical details and then on connect to enable the user report?

  1. As far as I understand, terms and conditions are contracts. For exampel terms and conditions on wikipedia is forwarded to https://en.wikipedia.org/wiki/Contractual_term
  1. GDPR says Where personal data relating to a data subject are collected from the data subject, the controller shall, at the time when personal data are obtained, provide the data subject with all of the following information. https://gdpr-info.eu/art-13-gdpr/

The button captions will be changed to Decline and Accept, Technical Details will become Terms as I originally seeked confirmation for new strings and had reused existing ones until including rP21887...

Basically it's the main purpose of the committed patches.

In D1627#65048, @smiley wrote:
In D1627#65047, @Imarok wrote:

Good job. 👍
Is it intended, that the Enable Feedback button is first greyed out and the user has to click on technical details and then on connect to enable the user report?

Pretty sure it's intended. They are supposed to read and accept the terms before doing it I guess.

Sure but wouldn't it be better to just open the terms window when the user clicks the enable feedback button without reading the terms instead of disabling the button?
The current way seems to be a bit counterintuitive.

Make sure that you judge this with the new strings, "Accept" "Decline" "Terms" and the tooltip Please read and accept the UserReporter Terms and Conditions for the disabled "Enable" button.
https://github.com/elexis1/0ad/commit/17ad4108e5d8897a29a93b1e0a81582e9aa0639a#diff-b61006c42467ca91caa61e77f2d890be

The use case for having the two buttons if the terms were not accepted is to be able to read the terms without necessarily intending to enable the service. If the button says "Enable", the user will think that it will enable the service and is not aware that he may read the description and terms prior to deciding that.
The use case for having the two buttons if the terms were accepted is to be able to read the terms again when wished for and to be able to disable the service.

Offtopic btw, as this stuff comes from https://github.com/bb-bb/0ad/commit/62acfd74cb3efdb517db537bd4e62fa0ba6c304d / rP21887 / D1602, not D1627 / rP21890 (and was available for participation and feedback for 7 weeks).