Code review comment for lp://qastaging/~justinmcp/unity-webapps-yahoonews/update-webapp

Revision history for this message
David Barth (dbarth) wrote :

Looks good to me, but please check the jenkins error to see if the GJS
complains are real or not.

On Wed, Feb 11, 2015 at 8:39 AM, Justin McPherson <
<email address hidden>> wrote:

> Justin McPherson has proposed merging
> lp:~justinmcp/unity-webapps-yahoonews/update-webapp into
> lp:unity-webapps-yahoonews.
>
> Commit message:
> Update Yahoonews app.
>
> - Fix homepage entry in manifest.json
> - Add support for updated page layout
>
> Requested reviews:
> WebApps (webapps)
>
> For more details, see:
>
> https://code.launchpad.net/~justinmcp/unity-webapps-yahoonews/update-webapp/+merge/249283
> --
> Your team WebApps is requested to review the proposed merge of
> lp:~justinmcp/unity-webapps-yahoonews/update-webapp into
> lp:unity-webapps-yahoonews.
>
> === modified file 'YahooNews.user.js'
> --- YahooNews.user.js 2013-06-18 07:37:48 +0000
> +++ YahooNews.user.js 2015-02-11 07:38:27 +0000
> @@ -13,6 +13,10 @@
> window.Unity = external.getUnityObject(1);
>
> function isCorrectPage() {
> + if (document.evaluate('//*[@id="Main"]', document, null,
> XPathResult.ORDERED_NODE_SNAPSHOT_TYPE, null).snapshotLength == 1) {
> + return true;
> + }
> +
> if (!document.getElementById('mediamegatron') &&
> !document.getElementById('mediablistmixednewsforyoucatemp') &&
> !document.getElementById('mediatabs')) {
> return false;
> }
> @@ -34,6 +38,17 @@
> res = document.evaluate('//ul[@class="tpl-thumb_100x75_title
> yom-list yom-list-large"]/li/div/a', document, null,
> XPathResult.ORDERED_NODE_SNAPSHOT_TYPE, null);
> }
>
> + if (res.snapshotLength == 0) {
> + var mainSection = document.evaluate('//*[@id="Main"]', document,
> null, XPathResult.ORDERED_NODE_SNAPSHOT_TYPE, null);
> + if (mainSection.snapshotLength == 1) {
> + res =
> document.evaluate('div/div/div/div/*/ul/li/div/div/div/*/a',
> + mainSection.snapshotItem(0),
> + null,
> +
> XPathResult.ORDERED_NODE_SNAPSHOT_TYPE,
> + null);
> + }
> + }
> +
> for (i = 0; i < res.snapshotLength; i++) {
> var node = res.snapshotItem(i);
> var title = node.textContent;
>
> === modified file 'manifest.json'
> --- manifest.json 2013-05-03 06:10:42 +0000
> +++ manifest.json 2015-02-11 07:38:27 +0000
> @@ -1,1 +1,1 @@
> -{"includes":["http://news.yahoo.com/","http://mx.noticias.yahoo.com/","
> http://es.noticias.yahoo.com/"],"requires":["utils.js"],"name":"YahooNews","scripts":["YahooNews.user.js"],"maintainer":"Webapps
> Team <<email address hidden>
> >","manifest-version":"1.0","integration-version":"2.3","package-name":"YahooNews","icons":{"128":"128/unity-webapps-yahoonews.png","48":"48/unity-webapps-yahoonews.png","52":"52/unity-webapps-yahoonews.png","64":"64/unity-webapps-yahoonews.png"},"domain":"
> yahoo.com","homepage":"news.yahoo.com","license":"GPL-3"}
> +{"includes":["http://news.yahoo.com/","http://mx.noticias.yahoo.com/","
> http://es.noticias.yahoo.com/"],"requires":["utils.js"],"name":"YahooNews","scripts":["YahooNews.user.js"],"maintainer":"Webapps
> Team <<email address hidden>
> >","manifest-version":"1.0","integration-version":"2.3","package-name":"YahooNews","icons":{"128":"128/unity-webapps-yahoonews.png","48":"48/unity-webapps-yahoonews.png","52":"52/unity-webapps-yahoonews.png","64":"64/unity-webapps-yahoonews.png"},"domain":"
> yahoo.com","homepage":"http://news.yahoo.com","license":"GPL-3"}
>
>
>

« Back to merge proposal