Merge lp:~justinmcp/unity-webapps-gmail/validation-update into lp:unity-webapps-gmail

[Justin McPherson]

Make use of node validations functions.

[David Barth (dbarth)]

LGTFM

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:89
http://jenkins.qa.ubuntu.com/job/unity-webapps-gmail-ci/17/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-webapps-gmail-trusty-amd64-ci/10/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-webapps-gmail-trusty-armhf-ci/10/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-webapps-gmail-ci/17/rebuild

review: Needs Fixing (continuous-integration)

[Maxim Ermilov (zaspire)]

> +// @require validate.js
require newer version of unity-webapps-common.
SRU will not allow dependency between updated packages

review: Disapprove

[David Barth (dbarth)]

Le 18/12/2013 16:57, Maxim Ermilov a écrit :
> Review: Disapprove
>
>> +// @require validate.js
> require newer version of unity-webapps-common.
> SRU will not allow dependency between updated packages
I have the impression you meant "needs fixing" rather than a strict
"disapprove", so would you have recommendations to make this compatible
with the SRU process?

If not, then we should limit that to the upcoming LTS, for which those
changes are made anyway.

David

[Alexandre Abreu (abreu-alexandre)]

> > +// @require validate.js
> require newer version of unity-webapps-common.
> SRU will not allow dependency between updated packages

This is a review for the *trunk* series (which should be called "14.04
") not for 13.10 or before ... so the SRU doe snot apply, I see no reason to Disapprove this ...

[Alexandre Abreu (abreu-alexandre)]

> > > +// @require validate.js
> > require newer version of unity-webapps-common.
> > SRU will not allow dependency between updated packages
>
> This is a review for the *trunk* series (which should be called "14.04
> ") not for 13.10 or before ... so the SRU doe snot apply, I see no reason to
> Disapprove this ...

... and we are focusing on 14.04 ....

[Alexandre Abreu (abreu-alexandre)]

This MR can only be further approved once this one lands:

https://code.launchpad.net/~justinmcp/webapps-applications/webapp-support/+merge/199408

review: Needs Fixing

[Robert Bruce Park (robru)]

It's true this can't be SRU'd but it looks fine to me for Trusty at least.

review: Approve

[Robert Bruce Park (robru)]

Ok Justin, I have a lesson in dependency tracking for you ;-)

So I landed both of your webapps-application merges and built them in the PPA, where it received version number 2.4.17+14.04.20140122-0ubuntu1. So now on this branch, go into debian/control and set the required version in the dependencies. So that means change line 16 from this:

Depends: unity-webapps-common,

to this:

Depends: unity-webapps-common (>= 2.4.17+14.04.20140122-0ubuntu1),

Then once you commit and push that to this branch, jenkins will re-review automatically and it should pass.

review: Approve

[lp:~justinmcp/unity-webapps-gmail/validation-update]

90. By Justin McPherson on 2014-01-23

Specifiy version of unity-webapps-common.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:90
http://jenkins.qa.ubuntu.com/job/unity-webapps-gmail-ci/21/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-webapps-gmail-trusty-amd64-ci/14
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-webapps-gmail-trusty-armhf-ci/14

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-webapps-gmail-ci/21/rebuild

review: Approve (continuous-integration)

[Robert Bruce Park (robru)]

Ok, so, I finally had some extra time to get my testing VMs back up to speed here (they'd bitrotted somewhat over the holidays).

Using a fresh trusty VM, I was able to test your branches much more thoroughly.

First, I installed webapps-applications from the PPA (eg, the latest unreleased version that contains your last two branches that merged), then I branched this branch, built and installed it.

I played around with it a bit, and saw that it is working well. The launcher has the correct 'active pip', switching works as expected, etc, looks like a good app.

So I'm going to approve this one.

review: Approve

=== modified file 'GMail.user.js'
--- GMail.user.js	2013-11-06 00:52:12 +0000
+++ GMail.user.js	2014-01-23 02:06:05 +0000
@@ -1,6 +1,7 @@
 // ==UserScript==
 // @include        https://mail.google.com/*
 // @require        utils.js
+// @require        validate.js
 // @require        google-common.js
 // ==/UserScript==
 
@@ -10,11 +11,7 @@
 ));
 
 window.Unity = external.getUnityObject(1);
-var pane = null;
 
-function getComposeMessageNode() {
-    return document.evaluate('//div[@role="navigation"]/div[1]/div[1]/div[1][@role="button"]', document, null, XPathResult.ANY_UNORDERED_NODE_TYPE, null).singleNodeValue;
-}
 
 function getNumber(str) {
     try {
@@ -26,40 +23,86 @@
     }
 }
 
+
+var interestNodes = {
+    login: {
+        name: 'login',
+        query: '//div[@role="navigation"]//*[contains(text(), "@")]',
+        validator: function (node) {
+            return node.textContent.match(/^.*@.*\.{1}.*$/) !== null;
+        },
+        fragment: '<div role="navigation"><span>[e-mail]</span></div>'
+    },
+    compose: {
+        name: "compose button",
+        query: '//div[@role="navigation"]/div[1]/div[1]/div[1][@role="button"]',
+        fragment: ''
+    },
+    labels: {
+        name: 'Labels',
+        query: '//div[@role="navigation"]/div[2]//a[contains(@href, "#label")]',
+        fragment: '',
+        value: function (node) {
+            return getNumber(node.textContent);
+        }
+    },
+    inbox: {
+        name: "Inbox",
+        query: '//div[@role="navigation"]/div[2]//a[contains(@href, "#inbox")]',
+        fragment: '',
+        value: function (node) {
+            return getNumber(node.text);
+        }
+    },
+    pane: {
+        name: 'pane',
+        query: '//div[@role="navigation"]'
+    }
+};
+
+function getComposeMessageNode() {
+    return validatedNode(interestNodes.compose);
+}
+
+
 function getLabels() {
     var i, res = [];
 
-    var snapshot = document.evaluate('//div[@role="navigation"]/div[2]//a[contains(@href, "#label")]',
-                                     document, null, XPathResult.UNORDERED_NODE_SNAPSHOT_TYPE, null);
-
-    for (i = 0; i < snapshot.snapshotLength; i++) {
-        var node = snapshot.snapshotItem(i);
-
-        res.push({ name: unescape(node.href.match(/#label\/(.+)$/)[1]),
-                   count: getNumber(node.textContent),
-                   link: node.href });
+    var labelNodes = validatedNodes(interestNodes.labels);
+
+    for (i = 0; i < labelNodes.length; i++) {
+        var node = labelNodes[i];
+
+        try {
+            res.push({ name: unescape(node.href.match(/#label\/(.+)$/)[1]),
+                       count: validatedNodeValue(interestNodes.labels, node),
+                       link: node.href });
+        } catch (labels_error) {
+        }
     }
 
     return res;
 }
 
 function checkMessagesCount() {
-
     var indicators = [];
 
-    var inboxLink = document.evaluate('//div[@role="navigation"]/div[2]//a[contains(@href, "#inbox")]', document, null, XPathResult.ANY_UNORDERED_NODE_TYPE, null).singleNodeValue;
-
-    if (!inboxLink || !inboxLink.href || !inboxLink.text) {
-        return indicators;
-    }
-    var numMessages = getNumber(inboxLink.text);
-
-    indicators.push({ name: _("Inbox"),
-                      count: numMessages,
-                      callback: makeRedirector(inboxLink.href) });
-    var i, labels = getLabels();
-    for (i = 0; i < labels.length; i++) {
-        indicators.push({ name: trim(labels[i].name), count: labels[i].count, callback: makeRedirector(labels[i].link) });
+    try {
+        var inboxNode = validatedNodeValue(interestNodes.inbox);
+        if (inboxNode === null) {
+            return indicators;
+        }
+
+        indicators.push({ name: _("Inbox"),
+                          count: validatedNodeValue(interestNodes.inbox, inboxNode),
+                          callback: makeRedirector(inboxNode.href) });
+
+        var i, labels = getLabels();
+        for (i = 0; i < labels.length; i++) {
+            indicators.push({ name: trim(labels[i].name), count: labels[i].count, callback: makeRedirector(labels[i].link) });
+        }
+
+    } catch (messages_error) {
     }
 
     return indicators;
@@ -80,23 +123,6 @@
     reportTestState('PASS SELF TEST');
 }
 
-function validatedNode(nodeInfo) {
-    var resultSet = document.evaluate(nodeInfo.query,
-                                      document,
-                                      null,
-                                      XPathResult.ORDERED_NODE_SNAPSHOT_TYPE,
-                                      null);
-    if (!nodeInfo.validator(resultSet.snapshotItem(0))) {
-        var testNode = document.createElement("div");
-        testNode.innerHTML = nodeInfo.fragment;
-        resultSet = document.evaluate(nodeInfo.query, testNode, null, XPathResult.ORDERED_NODE_SNAPSHOT_TYPE, null);
-        var fragmentOk = resultSet.snapshotLength > 0 && nodeInfo.validator(resultSet.snapshotItem(0));
-        reportTestState("REPORT: ERROR: Failed to validate node " + nodeInfo.name + " fragment check " + (fragmentOk ? "OK" : "failed"));
-        return null;
-    }
-    return resultSet.snapshotItem(0);
-}
-
 function isCorrectPage() {
     var i, ids = ['hist_frame', 'js_frame'];
 
@@ -149,10 +175,10 @@
     selfTest();
 }
 
+
 if (isCorrectPage()) {
     setTimeout(wrapCallback(function wait() {
-        pane = document.evaluate('//div[@role="navigation"]',
-                                 document, null, XPathResult.ANY_UNORDERED_NODE_TYPE, null).singleNodeValue;
+        var pane = validatedNode(interestNodes.pane);
 
         if (!pane || !document.getElementsByClassName("nU")) {
             setTimeout(wait, 1000);
@@ -160,15 +186,7 @@
         }
         var login = null;
         try {
-            var loginNode = validatedNode({
-                    name: 'login query',
-                    query: '//div[@role="navigation"]//*[contains(text(), "@")]',
-                    validator: function (node) {
-                        return node.textContent.match(/^.*@.*\.{1}.*$/) !== null;
-                    },
-                    fragment: '<div role="navigation"><span>[e-mail]</span></div>'
-                });
-
+            var loginNode = validatedNode(interestNodes.login);
             if (loginNode !== null) {
                 login = loginNode.textContent;
             }

=== modified file 'debian/control'
--- debian/control	2013-09-17 06:50:06 +0000
+++ debian/control	2014-01-23 02:06:05 +0000
@@ -13,7 +13,7 @@
 
 Package: unity-webapps-gmail
 Architecture: all
-Depends: unity-webapps-common,
+Depends: unity-webapps-common (>= 2.4.17+14.04.20140122-0ubuntu1),
          xdg-utils,
          ${misc:Depends},
 XB-Ubuntu-Webapps-Includes: https://mail.google.com/*

=== modified file 'manifest.json'
--- manifest.json	2013-11-05 01:16:47 +0000
+++ manifest.json	2014-01-23 02:06:05 +0000
@@ -1,1 +1,1 @@
-{"includes":["https://mail.google.com/*", "https://accounts.google.*/*", "https://www.google.com/a/*"],"requires":["utils.js","google-common.js"],"name":"Gmail","scripts":["GMail.user.js"],"maintainer":"Webapps Team <[e-mail]>","license":"GPL-3","manifest-version":"1.0","integration-version":"2.4.8","package-name":"Gmail","description":"Unity Webapp for Gmail","icons":{"128":"128/unity-webapps-gmail.png","48":"48/unity-webapps-gmail.png","52":"52/unity-webapps-gmail.png","64":"64/unity-webapps-gmail.png"},"domain":"mail.google.com","homepage":"https://mail.google.com"}
+{"includes":["https://mail.google.com/*", "https://accounts.google.*/*", "https://www.google.com/a/*"],"requires":["utils.js","google-common.js","validate.js"],"name":"Gmail","scripts":["GMail.user.js"],"maintainer":"Webapps Team <[e-mail]>","license":"GPL-3","manifest-version":"1.0","integration-version":"2.4.8","package-name":"Gmail","description":"Unity Webapp for Gmail","icons":{"128":"128/unity-webapps-gmail.png","48":"48/unity-webapps-gmail.png","52":"52/unity-webapps-gmail.png","64":"64/unity-webapps-gmail.png"},"domain":"mail.google.com","homepage":"https://mail.google.com"}