Add LiveZilla Tag #95

scysys · 2018-10-02T17:38:42Z

Add LiveZilla Tag

Tested and working.
Using it a few days on own Websites.

I think it has all options that are needed.

Findus23

Hi,

many thanks for creating a new Tag.

I have a few comments below (keep in mind that I don't know LiveZilla)

Please also fix the indention as it seems to be a bit off in a few lines.

Template/Tag/LivezillaDynamicTag.php

Findus23 · 2018-10-02T17:56:06Z

Template/Tag/LivezillaDynamicTag.php

+        // return parent::getIcon();
+        //
+        // The image should have ideally a resolution of about 64x64 pixels.
+        return 'plugins/TagManager/images/icons/livezilla_icon.png';


I wanted to complain about the ugly icon, but it seems like you took the best they have
🙂

I try to find another Image, but im failed :(

Template/Tag/LivezillaDynamicTag.web.js

Findus23 · 2018-10-02T17:59:34Z

Template/Tag/LivezillaDynamicTag.php

+            }),
+			$this->makeSetting('LivezillaDynamicDomain', '', FieldConfig::TYPE_STRING, function (FieldConfig $field) {
+                $field->title = 'Livezilla Domain';
+                $field->description = 'Enter your Domain without http:// or https://';


While obviously everyone should be using https, it is a bit counter intuitive that https is hardcoded.

Livezilla is an Chat and Monitoring solution. Thoose things violates privacy of an website visitor. I do not tolerate those implementations over an insecure http connection!

I fully agree, then you should make it clear to the user (e.g. make them enter the URL and throw an error if it isn't HTTPS).

If you want to I can help you write this validate function.

Findus23 · 2018-10-02T18:02:03Z

Template/Tag/LivezillaDynamicTag.php

+                $field->description = 'In most cases it should be the Standard. (true)
+				Available options are: true or false'; // Howto linebreak?
+				$field->validators[] = new NotEmpty();
+                $field->validators[] = new CharacterLength(4, 5);


Please use a Checkbox for boolean settings instead of saving a string.

See here for an example:

tag-manager/Template/Tag/RaygunTag.php

Lines 29 to 33 in 5649e87

$this->makeSetting('raygunEnablePulse', false, FieldConfig::TYPE_BOOL, function (FieldConfig $field) {

$field->title = 'Enable Pulse (Real User Monitoring)';

$field->uiControl = FieldConfig::UI_CONTROL_CHECKBOX;

$field->description = 'Automatically identify front end performance issues causing slow page load speeds. See what your users see in the browser and discover why users had poor quality experiences.';

})

Findus23 · 2018-10-02T18:10:50Z

Template/Tag/LivezillaDynamicTag.php

+				$field->validators[] = new NotEmpty();
+                $field->validators[] = new CharacterLength(4, 5);
+            }),
+			$this->makeSetting('LivezillaDynamicCookieLawName', '', FieldConfig::TYPE_STRING, function (FieldConfig $field) {


I think the cookie feature makes this tag unnecessarily complicated and hard to understand.
@tsteur What do you think about adding a cookie feature (trigger?) to MTM?

I using it with an other implantation for the tag-manager. It respect the Privacy of European Users and load this Tag only when he really accept those parts on an website.

The tag-manager gives me no real way to document this for other users.

Findus23 · 2018-10-02T18:18:02Z

Template/Tag/LivezillaDynamicTag.web.js

+			if (document.cookie.indexOf('' + LivezillaDynamicCookieLawName + '=' + LivezillaDynamicCookieLawValue + '') > -1 ) {
+                var s1 = document.createElement("script"), s0 = document.getElementsByTagName("script")[0];
+				s1.type = 'text/javascript';
+				s1.defer = LivezillaDynamicDefer;


This is the reason you never store boolean values as a string.
Even in Javascript "false" is true/truthy which means that if a user had entered "false" into the settings it would still be deferred.
Because this returns this is truthy

if ("false") { console.log("this is truthy") } else { console.log("this is falsey") }

Findus23 · 2018-10-02T18:19:41Z

lang/de.json

@@ -278,6 +278,9 @@
        "LastUpdated": "Zuletzt aktualisiert",
        "LastVersions": "Letzte Versionen",
        "LearnMore": "Mehr dazu",
+		"LivezillaDynamicTagName": "Livezilla Chat / Monitoring",


Translations are managed via transifex, so please don't modify them here.

I did not knew that.

This reverts commit 9288123.

tsteur · 2018-10-03T19:32:22Z

Template/Tag/LivezillaDynamicTag.php

+                $field->validators[] = new NotEmpty();
+                $field->validators[] = new CharacterLength(32);
+            }),
+			$this->makeSetting('LivezillaDynamicDomain', '', FieldConfig::TYPE_STRING, function (FieldConfig $field) {


I'm not quite sure but could this be maybe automatically configured in the JavaScript depending on the current domain? eg var LivezilleDomain = localhost.origin or so?

@tsteur I am not sure, but as I understood it this is the URL where Livezilla is installed (so similar to the Matomo URL)

I see. Makes sense 👍

Findus23

Appart from the question on how to best handle opt-out/opt-in cookies, this looks good to me now.

@tsteur, can you please take a closer look.

tsteur · 2018-10-04T20:18:53Z

Template/Tag/LivezillaDynamicTag.php

+			$this->makeSetting('LivezillaDynamicDefer', true, FieldConfig::TYPE_BOOL, function (FieldConfig $field) {
+                $field->title = 'Livezilla Script "defer"?';
+				$field->uiControl = FieldConfig::UI_CONTROL_CHECKBOX;
+                $field->description = 'In most cases you should let it activated.';


If possible, be good to give an example when it makes sense to disable it. Not needed though.

No its really not needed. But sometimes you have sites where you would remove this part. So there are 2 Options. Remove "defer" completely or leave it as default and give an user no options to remove it.

An really better solution would be an general option in Tag Manager to async or defer scripts. I don't find those options at this moment.

Example from Qubit´s OpenTag Manager

tsteur · 2018-10-04T20:19:34Z

@scysys @Findus23 looks good to me. I would only replace all tabs with spaces and sometimes it looks like there are two many tabs/spaces.

tsteur · 2018-10-04T20:19:46Z

Tested it and worked fine for me.

tsteur

Cheers 👍 I'll merge soon

Add LiveZilla Tag

9288123

Findus23 added the New Tag Adding a new Tag label Oct 2, 2018

Findus23 requested changes Oct 2, 2018

View reviewed changes

scysys added 2 commits October 2, 2018 21:26

Revert "Add LiveZilla Tag"

26672a4

This reverts commit 9288123.

Add Livezilla Tag

5ac4add

tsteur reviewed Oct 3, 2018

View reviewed changes

Findus23 approved these changes Oct 4, 2018

View reviewed changes

Findus23 added the Needs Review label Oct 4, 2018

tsteur reviewed Oct 4, 2018

View reviewed changes

Reformat Code

45e3e1d

tsteur approved these changes Oct 7, 2018

View reviewed changes

tsteur merged commit 807883a into matomo-org:master Nov 4, 2018

	$this->makeSetting('raygunEnablePulse', false, FieldConfig::TYPE_BOOL, function (FieldConfig $field) {
	$field->title = 'Enable Pulse (Real User Monitoring)';
	$field->uiControl = FieldConfig::UI_CONTROL_CHECKBOX;
	$field->description = 'Automatically identify front end performance issues causing slow page load speeds. See what your users see in the browser and discover why users had poor quality experiences.';
	})

Add LiveZilla Tag #95

Add LiveZilla Tag #95

Uh oh!

Conversation

scysys commented Oct 2, 2018

Uh oh!

Findus23 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Findus23 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scysys Oct 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tsteur commented Oct 4, 2018

Uh oh!

tsteur commented Oct 4, 2018

Uh oh!

tsteur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

scysys Oct 5, 2018 •

edited

Loading