Uploaded image for project: 'Apache Cordova'
  1. Apache Cordova
  2. CB-12804

Cordova-browser PWA needs a manifest file

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: cordova7
    • Component/s: cordova-browser
    • Labels:
      None

      Description

      Cordova-browser PWA needs a manifest file.
      This manifest.json file should get created during cordova platform add browser and use values from config.xml to build.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user purplecabbage commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/32#discussion_r124158440

          — Diff: bin/template/www/cordova-sw.js —
          @@ -0,0 +1,23 @@
          +
          +// Note, these will be updated automatically at build time
          +var CACHE_VERSION = '%CACHE_VERSION%';
          +var CACHE_LIST = ['CACHE_VALUES'];
          +
          +this.addEventListener('install', function(event) {
          + // Perform install steps
          + console.log("cordova service worker is installing.");
          + event.waitUntil(caches.open(CACHE_VERSION)
          + .then(function(cache)

          { + return cache.addAll(CACHE_LIST); + }

          ));
          +});
          +
          +this.addEventListener('activate', function(event)

          { + // Perform activate steps + console.log("cordova service worker is activated."); +}

          );
          +
          +this.addEventListener('fetch', function(event) {
          + console.log("cordova service worker : fetch : " + event.request.url);
          + event.respondWith(caches.match(event.request));
          — End diff –

          defensively added to #36

          Show
          githubbot ASF GitHub Bot added a comment - Github user purplecabbage commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/32#discussion_r124158440 — Diff: bin/template/www/cordova-sw.js — @@ -0,0 +1,23 @@ + +// Note, these will be updated automatically at build time +var CACHE_VERSION = '%CACHE_VERSION%'; +var CACHE_LIST = ['CACHE_VALUES'] ; + +this.addEventListener('install', function(event) { + // Perform install steps + console.log("cordova service worker is installing."); + event.waitUntil(caches.open(CACHE_VERSION) + .then(function(cache) { + return cache.addAll(CACHE_LIST); + } )); +}); + +this.addEventListener('activate', function(event) { + // Perform activate steps + console.log("cordova service worker is activated."); +} ); + +this.addEventListener('fetch', function(event) { + console.log("cordova service worker : fetch : " + event.request.url); + event.respondWith(caches.match(event.request)); — End diff – defensively added to #36
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user purplecabbage commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/32#discussion_r124158393

          — Diff: bin/template/cordova/Api.js —
          @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () {
          "locations":this.locations,
          "root": this.root,
          "name": this.platform,

          • "version": { "version" : "1.0.0" }

            ,
            + "version":

            { "version" : "1.0.0" }

            , // um, todo!
            "projectConfig": this.config
            };
            };

          Api.prototype.prepare = function (cordovaProject,options) {

          // First cleanup current config and merge project's one into own

          • var defaultConfig = path.join(this.locations.platformRootDir,'cordova',
            + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova',
            'defaults.xml');
            -
          • var ownConfig = this.locations.configXml;
            -
            + var ownConfigPath = this.locations.configXml;
            var sourceCfg = cordovaProject.projectConfig;
            +
            // If defaults.xml is present, overwrite platform config.xml with it.
            // Otherwise save whatever is there as defaults so it can be
            // restored or copy project config into platform if none exists.
          • if (fs.existsSync(defaultConfig)) {
            + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - }

            else if (fs.existsSync(ownConfig))

            { - shell.cp('-f', ownConfig, defaultConfig); - }

            else

            { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + }

            + else if (fs.existsSync(ownConfigPath))

            { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + }

            + else

            { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); }
          • // this._munger.reapply_global_munge().save_all();
            -
          • this.config = new ConfigParser(ownConfig);
            + // merge our configs
            + this.config = new ConfigParser(ownConfigPath);
            xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(),
          • this.config.doc.getroot(), this.platform, true);
            + this.config.doc.getroot(),
            + this.platform, true);
            this.config.write();
          • /*
          • "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - }
          • */
            -
            // Update own www dir with project's www assets and plugins' assets and js-files
            this.parser.update_www(cordovaProject.locations.www);

          + // Copy or Create manifest.json
          + // todo: move this to a manifest helper module
          + // output path
          + var manifestPath = path.join(this.locations.www,'manifest.json');
          + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json');
          + if(fs.existsSync(srcManifestPath))

          { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + }

          + else {
          + var manifestJson =

          { + "background_color": "#000", + "display": "standalone" + }

          ;
          + if(this.config){
          + if(this.config.name())

          { + manifestJson.name = this.config.name(); + }

          + if(this.config.shortName())

          { + manifestJson.short_name = this.config.shortName(); + }

          + if(this.config.packageName())

          { + manifestJson.version = this.config.packageName(); + }

          + if(this.config.description())

          { + manifestJson.description = this.config.description(); + }

          + if(this.config.author())

          { + manifestJson.author = this.config.author(); + }

          + // icons
          + var icons = this.config.getStaticResources('browser','icon');
          + var manifestIcons = icons.map(function(icon) {
          + // given a tag like this :
          + // <icon src="res/ios/icon.png" width="57" height="57" density="mdpi" />
          + /* configParser returns icons that look like this :
          +

          { src: 'res/ios/icon.png', + target: undefined, + density: 'mdpi', + platform: null, + width: 57, + height: 57 + }

          ******/
          + /* manifest expects them to be like this :
          +

          { "src": "images/touch/icon-128x128.png", + "type": "image/png", + "sizes": "128x128" + }

          ******/
          + // ?Is it worth looking at file extentions?
          + return

          {"src":icon.src, "type":"image/png", + "sizes":(icon.width + "x" + icon.height)}

          ;
          + });
          + manifestJson.icons = manifestIcons;
          +
          + // orientation
          + // <preference name="Orientation" value="landscape" />
          + var oriPref = this.config.getGlobalPreference('Orientation');
          + if(oriPref && ["landscape","portrait"].indexOf(oriPref) > -1)

          { + manifestJson.orientation = oriPref; + }

          — End diff –

          all other values will be mapped to 'any' in #36

          Show
          githubbot ASF GitHub Bot added a comment - Github user purplecabbage commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/32#discussion_r124158393 — Diff: bin/template/cordova/Api.js — @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () { "locations":this.locations, "root": this.root, "name": this.platform, "version": { "version" : "1.0.0" } , + "version": { "version" : "1.0.0" } , // um, todo! "projectConfig": this.config }; }; Api.prototype.prepare = function (cordovaProject,options) { // First cleanup current config and merge project's one into own var defaultConfig = path.join(this.locations.platformRootDir,'cordova', + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova', 'defaults.xml'); - var ownConfig = this.locations.configXml; - + var ownConfigPath = this.locations.configXml; var sourceCfg = cordovaProject.projectConfig; + // If defaults.xml is present, overwrite platform config.xml with it. // Otherwise save whatever is there as defaults so it can be // restored or copy project config into platform if none exists. if (fs.existsSync(defaultConfig)) { + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - } else if (fs.existsSync(ownConfig)) { - shell.cp('-f', ownConfig, defaultConfig); - } else { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + } + else if (fs.existsSync(ownConfigPath)) { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + } + else { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); } // this._munger.reapply_global_munge().save_all(); - this.config = new ConfigParser(ownConfig); + // merge our configs + this.config = new ConfigParser(ownConfigPath); xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(), this.config.doc.getroot(), this.platform, true); + this.config.doc.getroot(), + this.platform, true); this.config.write(); /* "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - } */ - // Update own www dir with project's www assets and plugins' assets and js-files this.parser.update_www(cordovaProject.locations.www); + // Copy or Create manifest.json + // todo: move this to a manifest helper module + // output path + var manifestPath = path.join(this.locations.www,'manifest.json'); + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json'); + if(fs.existsSync(srcManifestPath)) { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + } + else { + var manifestJson = { + "background_color": "#000", + "display": "standalone" + } ; + if(this.config){ + if(this.config.name()) { + manifestJson.name = this.config.name(); + } + if(this.config.shortName()) { + manifestJson.short_name = this.config.shortName(); + } + if(this.config.packageName()) { + manifestJson.version = this.config.packageName(); + } + if(this.config.description()) { + manifestJson.description = this.config.description(); + } + if(this.config.author()) { + manifestJson.author = this.config.author(); + } + // icons + var icons = this.config.getStaticResources('browser','icon'); + var manifestIcons = icons.map(function(icon) { + // given a tag like this : + // <icon src="res/ios/icon.png" width="57" height="57" density="mdpi" /> + /* configParser returns icons that look like this : + { src: 'res/ios/icon.png', + target: undefined, + density: 'mdpi', + platform: null, + width: 57, + height: 57 + } ******/ + /* manifest expects them to be like this : + { "src": "images/touch/icon-128x128.png", + "type": "image/png", + "sizes": "128x128" + } ******/ + // ?Is it worth looking at file extentions? + return {"src":icon.src, "type":"image/png", + "sizes":(icon.width + "x" + icon.height)} ; + }); + manifestJson.icons = manifestIcons; + + // orientation + // <preference name="Orientation" value="landscape" /> + var oriPref = this.config.getGlobalPreference('Orientation'); + if(oriPref && ["landscape","portrait"] .indexOf(oriPref) > -1) { + manifestJson.orientation = oriPref; + } — End diff – all other values will be mapped to 'any' in #36
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user purplecabbage commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/32#discussion_r124158333

          — Diff: bin/template/cordova/Api.js —
          @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () {
          "locations":this.locations,
          "root": this.root,
          "name": this.platform,

          • "version": { "version" : "1.0.0" }

            ,
            + "version":

            { "version" : "1.0.0" }

            , // um, todo!
            "projectConfig": this.config
            };
            };

          Api.prototype.prepare = function (cordovaProject,options) {

          // First cleanup current config and merge project's one into own

          • var defaultConfig = path.join(this.locations.platformRootDir,'cordova',
            + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova',
            'defaults.xml');
            -
          • var ownConfig = this.locations.configXml;
            -
            + var ownConfigPath = this.locations.configXml;
            var sourceCfg = cordovaProject.projectConfig;
            +
            // If defaults.xml is present, overwrite platform config.xml with it.
            // Otherwise save whatever is there as defaults so it can be
            // restored or copy project config into platform if none exists.
          • if (fs.existsSync(defaultConfig)) {
            + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - }

            else if (fs.existsSync(ownConfig))

            { - shell.cp('-f', ownConfig, defaultConfig); - }

            else

            { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + }

            + else if (fs.existsSync(ownConfigPath))

            { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + }

            + else

            { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); }
          • // this._munger.reapply_global_munge().save_all();
            -
          • this.config = new ConfigParser(ownConfig);
            + // merge our configs
            + this.config = new ConfigParser(ownConfigPath);
            xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(),
          • this.config.doc.getroot(), this.platform, true);
            + this.config.doc.getroot(),
            + this.platform, true);
            this.config.write();
          • /*
          • "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - }
          • */
            -
            // Update own www dir with project's www assets and plugins' assets and js-files
            this.parser.update_www(cordovaProject.locations.www);

          + // Copy or Create manifest.json
          + // todo: move this to a manifest helper module
          + // output path
          + var manifestPath = path.join(this.locations.www,'manifest.json');
          + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json');
          + if(fs.existsSync(srcManifestPath))

          { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + }

          + else {
          + var manifestJson = {
          + "background_color": "#000",
          — End diff –

          Changed to white in #36

          Show
          githubbot ASF GitHub Bot added a comment - Github user purplecabbage commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/32#discussion_r124158333 — Diff: bin/template/cordova/Api.js — @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () { "locations":this.locations, "root": this.root, "name": this.platform, "version": { "version" : "1.0.0" } , + "version": { "version" : "1.0.0" } , // um, todo! "projectConfig": this.config }; }; Api.prototype.prepare = function (cordovaProject,options) { // First cleanup current config and merge project's one into own var defaultConfig = path.join(this.locations.platformRootDir,'cordova', + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova', 'defaults.xml'); - var ownConfig = this.locations.configXml; - + var ownConfigPath = this.locations.configXml; var sourceCfg = cordovaProject.projectConfig; + // If defaults.xml is present, overwrite platform config.xml with it. // Otherwise save whatever is there as defaults so it can be // restored or copy project config into platform if none exists. if (fs.existsSync(defaultConfig)) { + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - } else if (fs.existsSync(ownConfig)) { - shell.cp('-f', ownConfig, defaultConfig); - } else { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + } + else if (fs.existsSync(ownConfigPath)) { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + } + else { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); } // this._munger.reapply_global_munge().save_all(); - this.config = new ConfigParser(ownConfig); + // merge our configs + this.config = new ConfigParser(ownConfigPath); xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(), this.config.doc.getroot(), this.platform, true); + this.config.doc.getroot(), + this.platform, true); this.config.write(); /* "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - } */ - // Update own www dir with project's www assets and plugins' assets and js-files this.parser.update_www(cordovaProject.locations.www); + // Copy or Create manifest.json + // todo: move this to a manifest helper module + // output path + var manifestPath = path.join(this.locations.www,'manifest.json'); + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json'); + if(fs.existsSync(srcManifestPath)) { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + } + else { + var manifestJson = { + "background_color": "#000", — End diff – Changed to white in #36
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/cordova-browser/pull/36

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/cordova-browser/pull/36
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user purplecabbage opened a pull request:

          https://github.com/apache/cordova-browser/pull/36

          CB-12804 Fixes some simple details - manifest.json

              1. Platforms affected
                browser
              1. What does this PR do?
                addresses issues raised with pr #32
              1. What testing has been done on this change?
                manual testing
              1. Checklist
          • [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
          • [x] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
          • [x] Added automated test coverage as appropriate for this change.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/purplecabbage/cordova-browser CB-12804-2

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/cordova-browser/pull/36.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #36


          commit 7aa99f6f19ec8cf4be4136242e42d36cb6ec460b
          Author: Jesse MacFadyen <purplecabbage@gmail.com>
          Date: 2017-06-27T01:07:20Z

          Approach cache more defensively

          commit b7ac532a06f3aea72bb45d5e6b8c7c36ed48cbcd
          Author: Jesse MacFadyen <purplecabbage@gmail.com>
          Date: 2017-06-27T01:08:21Z

          use white as default bgcolor in manifest, map unknown orientation values to 'any', address some issues raised in #32


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user purplecabbage opened a pull request: https://github.com/apache/cordova-browser/pull/36 CB-12804 Fixes some simple details - manifest.json Platforms affected browser What does this PR do? addresses issues raised with pr #32 What testing has been done on this change? manual testing Checklist [x] [Reported an issue] ( http://cordova.apache.org/contribute/issues.html ) in the JIRA database [x] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected. [x] Added automated test coverage as appropriate for this change. You can merge this pull request into a Git repository by running: $ git pull https://github.com/purplecabbage/cordova-browser CB-12804 -2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-browser/pull/36.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #36 commit 7aa99f6f19ec8cf4be4136242e42d36cb6ec460b Author: Jesse MacFadyen <purplecabbage@gmail.com> Date: 2017-06-27T01:07:20Z Approach cache more defensively commit b7ac532a06f3aea72bb45d5e6b8c7c36ed48cbcd Author: Jesse MacFadyen <purplecabbage@gmail.com> Date: 2017-06-27T01:08:21Z use white as default bgcolor in manifest, map unknown orientation values to 'any', address some issues raised in #32
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user purplecabbage commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/32#discussion_r124157183

          — Diff: cordova-js-src/confighelper.js —
          @@ -61,14 +61,9 @@ function readConfig(success, error) {
          }
          };

          • if ("ActiveXObject" in window) {
          • // Needed for XHR-ing via file:// protocol in IE
          • xhr = new window.ActiveXObject("MSXML2.XMLHTTP");
              • End diff –

          I'm older

          Show
          githubbot ASF GitHub Bot added a comment - Github user purplecabbage commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/32#discussion_r124157183 — Diff: cordova-js-src/confighelper.js — @@ -61,14 +61,9 @@ function readConfig(success, error) { } }; if ("ActiveXObject" in window) { // Needed for XHR-ing via file:// protocol in IE xhr = new window.ActiveXObject("MSXML2.XMLHTTP"); End diff – I'm older
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user purplecabbage commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/32#discussion_r124157156

          — Diff: bin/template/cordova/Api.js —
          @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () {
          "locations":this.locations,
          "root": this.root,
          "name": this.platform,

          • "version": { "version" : "1.0.0" }

            ,
            + "version":

            { "version" : "1.0.0" }

            , // um, todo!
            "projectConfig": this.config
            };
            };

          Api.prototype.prepare = function (cordovaProject,options) {

          // First cleanup current config and merge project's one into own

          • var defaultConfig = path.join(this.locations.platformRootDir,'cordova',
            + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova',
            'defaults.xml');
            -
          • var ownConfig = this.locations.configXml;
            -
            + var ownConfigPath = this.locations.configXml;
            var sourceCfg = cordovaProject.projectConfig;
            +
            // If defaults.xml is present, overwrite platform config.xml with it.
            // Otherwise save whatever is there as defaults so it can be
            // restored or copy project config into platform if none exists.
          • if (fs.existsSync(defaultConfig)) {
            + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - }

            else if (fs.existsSync(ownConfig))

            { - shell.cp('-f', ownConfig, defaultConfig); - }

            else

            { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + }

            + else if (fs.existsSync(ownConfigPath))

            { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + }

            + else

            { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); }
          • // this._munger.reapply_global_munge().save_all();
            -
          • this.config = new ConfigParser(ownConfig);
            + // merge our configs
            + this.config = new ConfigParser(ownConfigPath);
            xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(),
          • this.config.doc.getroot(), this.platform, true);
            + this.config.doc.getroot(),
            + this.platform, true);
            this.config.write();
          • /*
          • "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - }
          • */
            -
            // Update own www dir with project's www assets and plugins' assets and js-files
            this.parser.update_www(cordovaProject.locations.www);

          + // Copy or Create manifest.json
          + // todo: move this to a manifest helper module
          + // output path
          + var manifestPath = path.join(this.locations.www,'manifest.json');
          + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json');
          + if(fs.existsSync(srcManifestPath))

          { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + }

          + else {
          + var manifestJson =

          { + "background_color": "#000", + "display": "standalone" + }

          ;
          + if(this.config){
          + if(this.config.name())

          { + manifestJson.name = this.config.name(); + }

          + if(this.config.shortName())

          { + manifestJson.short_name = this.config.shortName(); + }

          + if(this.config.packageName())

          { + manifestJson.version = this.config.packageName(); + }

          + if(this.config.description())

          { + manifestJson.description = this.config.description(); + }

          + if(this.config.author())

          { + manifestJson.author = this.config.author(); + }

          + // icons
          + var icons = this.config.getStaticResources('browser','icon');
          + var manifestIcons = icons.map(function(icon) {
          + // given a tag like this :
          + // <icon src="res/ios/icon.png" width="57" height="57" density="mdpi" />
          + /* configParser returns icons that look like this :
          +

          { src: 'res/ios/icon.png', + target: undefined, + density: 'mdpi', + platform: null, + width: 57, + height: 57 + }

          ******/
          + /* manifest expects them to be like this :
          +

          { "src": "images/touch/icon-128x128.png", + "type": "image/png", + "sizes": "128x128" + }

          ******/
          + // ?Is it worth looking at file extentions?
          + return

          {"src":icon.src, "type":"image/png", + "sizes":(icon.width + "x" + icon.height)}

          ;
          + });
          + manifestJson.icons = manifestIcons;
          +
          + // orientation
          + // <preference name="Orientation" value="landscape" />
          + var oriPref = this.config.getGlobalPreference('Orientation');
          + if(oriPref && ["landscape","portrait"].indexOf(oriPref) > -1)

          { + manifestJson.orientation = oriPref; + }

          +
          + // get start_url
          + var contentNode = this.config.doc.find('content') || {'attrib':{'src':'index.html'}}; // sensible default
          + manifestJson.start_url = contentNode.attrib.src;
          +
          + // now we get some values from start_url page ...
          + var startUrlPath = path.join(cordovaProject.locations.www,manifestJson.start_url);
          + if(fs.existsSync(startUrlPath)) {
          + var contents = fs.readFileSync(startUrlPath, 'utf-8');
          + // matches <meta name="theme-color" content="#FF0044">
          + var themeColorRegex = /<meta(?=[^>]name="theme-color")\s[^>]*content="([^>])"/i;
          + var result = themeColorRegex.exec(contents);
          + var themeColor;
          + if(result && result.length>=2)

          { + themeColor = result[1]; + }

          + else { // see if there is a preference in config.xml
          + // <preference name="StatusBarBackgroundColor" value="#000000" />
          + themeColor = this.config.getGlobalPreference('StatusBarBackgroundColor');
          — End diff –

          Why would we ignore some values? What is wrong with a black statusbar?

          Show
          githubbot ASF GitHub Bot added a comment - Github user purplecabbage commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/32#discussion_r124157156 — Diff: bin/template/cordova/Api.js — @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () { "locations":this.locations, "root": this.root, "name": this.platform, "version": { "version" : "1.0.0" } , + "version": { "version" : "1.0.0" } , // um, todo! "projectConfig": this.config }; }; Api.prototype.prepare = function (cordovaProject,options) { // First cleanup current config and merge project's one into own var defaultConfig = path.join(this.locations.platformRootDir,'cordova', + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova', 'defaults.xml'); - var ownConfig = this.locations.configXml; - + var ownConfigPath = this.locations.configXml; var sourceCfg = cordovaProject.projectConfig; + // If defaults.xml is present, overwrite platform config.xml with it. // Otherwise save whatever is there as defaults so it can be // restored or copy project config into platform if none exists. if (fs.existsSync(defaultConfig)) { + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - } else if (fs.existsSync(ownConfig)) { - shell.cp('-f', ownConfig, defaultConfig); - } else { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + } + else if (fs.existsSync(ownConfigPath)) { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + } + else { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); } // this._munger.reapply_global_munge().save_all(); - this.config = new ConfigParser(ownConfig); + // merge our configs + this.config = new ConfigParser(ownConfigPath); xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(), this.config.doc.getroot(), this.platform, true); + this.config.doc.getroot(), + this.platform, true); this.config.write(); /* "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - } */ - // Update own www dir with project's www assets and plugins' assets and js-files this.parser.update_www(cordovaProject.locations.www); + // Copy or Create manifest.json + // todo: move this to a manifest helper module + // output path + var manifestPath = path.join(this.locations.www,'manifest.json'); + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json'); + if(fs.existsSync(srcManifestPath)) { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + } + else { + var manifestJson = { + "background_color": "#000", + "display": "standalone" + } ; + if(this.config){ + if(this.config.name()) { + manifestJson.name = this.config.name(); + } + if(this.config.shortName()) { + manifestJson.short_name = this.config.shortName(); + } + if(this.config.packageName()) { + manifestJson.version = this.config.packageName(); + } + if(this.config.description()) { + manifestJson.description = this.config.description(); + } + if(this.config.author()) { + manifestJson.author = this.config.author(); + } + // icons + var icons = this.config.getStaticResources('browser','icon'); + var manifestIcons = icons.map(function(icon) { + // given a tag like this : + // <icon src="res/ios/icon.png" width="57" height="57" density="mdpi" /> + /* configParser returns icons that look like this : + { src: 'res/ios/icon.png', + target: undefined, + density: 'mdpi', + platform: null, + width: 57, + height: 57 + } ******/ + /* manifest expects them to be like this : + { "src": "images/touch/icon-128x128.png", + "type": "image/png", + "sizes": "128x128" + } ******/ + // ?Is it worth looking at file extentions? + return {"src":icon.src, "type":"image/png", + "sizes":(icon.width + "x" + icon.height)} ; + }); + manifestJson.icons = manifestIcons; + + // orientation + // <preference name="Orientation" value="landscape" /> + var oriPref = this.config.getGlobalPreference('Orientation'); + if(oriPref && ["landscape","portrait"] .indexOf(oriPref) > -1) { + manifestJson.orientation = oriPref; + } + + // get start_url + var contentNode = this.config.doc.find('content') || {'attrib':{'src':'index.html'}}; // sensible default + manifestJson.start_url = contentNode.attrib.src; + + // now we get some values from start_url page ... + var startUrlPath = path.join(cordovaProject.locations.www,manifestJson.start_url); + if(fs.existsSync(startUrlPath)) { + var contents = fs.readFileSync(startUrlPath, 'utf-8'); + // matches <meta name="theme-color" content="#FF0044"> + var themeColorRegex = /<meta(?= [^>] name="theme-color")\s [^>] *content="( [^>] )"/i; + var result = themeColorRegex.exec(contents); + var themeColor; + if(result && result.length>=2) { + themeColor = result[1]; + } + else { // see if there is a preference in config.xml + // <preference name="StatusBarBackgroundColor" value="#000000" /> + themeColor = this.config.getGlobalPreference('StatusBarBackgroundColor'); — End diff – Why would we ignore some values? What is wrong with a black statusbar?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/32#discussion_r122782989

          — Diff: bin/template/cordova/Api.js —
          @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () {
          "locations":this.locations,
          "root": this.root,
          "name": this.platform,

          • "version": { "version" : "1.0.0" }

            ,
            + "version":

            { "version" : "1.0.0" }

            , // um, todo!
            "projectConfig": this.config
            };
            };

          Api.prototype.prepare = function (cordovaProject,options) {

          // First cleanup current config and merge project's one into own

          • var defaultConfig = path.join(this.locations.platformRootDir,'cordova',
            + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova',
            'defaults.xml');
            -
          • var ownConfig = this.locations.configXml;
            -
            + var ownConfigPath = this.locations.configXml;
            var sourceCfg = cordovaProject.projectConfig;
            +
            // If defaults.xml is present, overwrite platform config.xml with it.
            // Otherwise save whatever is there as defaults so it can be
            // restored or copy project config into platform if none exists.
          • if (fs.existsSync(defaultConfig)) {
            + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - }

            else if (fs.existsSync(ownConfig))

            { - shell.cp('-f', ownConfig, defaultConfig); - }

            else

            { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + }

            + else if (fs.existsSync(ownConfigPath))

            { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + }

            + else

            { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); }
          • // this._munger.reapply_global_munge().save_all();
            -
          • this.config = new ConfigParser(ownConfig);
            + // merge our configs
            + this.config = new ConfigParser(ownConfigPath);
            xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(),
          • this.config.doc.getroot(), this.platform, true);
            + this.config.doc.getroot(),
            + this.platform, true);
            this.config.write();
          • /*
          • "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - }
          • */
            -
            // Update own www dir with project's www assets and plugins' assets and js-files
            this.parser.update_www(cordovaProject.locations.www);

          + // Copy or Create manifest.json
          + // todo: move this to a manifest helper module
          + // output path
          + var manifestPath = path.join(this.locations.www,'manifest.json');
          + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json');
          + if(fs.existsSync(srcManifestPath))

          { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + }

          + else {
          + var manifestJson =

          { + "background_color": "#000", + "display": "standalone" + }

          ;
          + if(this.config){
          + if(this.config.name())

          { + manifestJson.name = this.config.name(); + }

          + if(this.config.shortName())

          { + manifestJson.short_name = this.config.shortName(); + }

          + if(this.config.packageName())

          { + manifestJson.version = this.config.packageName(); + }

          + if(this.config.description())

          { + manifestJson.description = this.config.description(); + }

          + if(this.config.author())

          { + manifestJson.author = this.config.author(); + }

          + // icons
          + var icons = this.config.getStaticResources('browser','icon');
          + var manifestIcons = icons.map(function(icon) {
          + // given a tag like this :
          + // <icon src="res/ios/icon.png" width="57" height="57" density="mdpi" />
          + /* configParser returns icons that look like this :
          +

          { src: 'res/ios/icon.png', + target: undefined, + density: 'mdpi', + platform: null, + width: 57, + height: 57 + }

          ******/
          + /* manifest expects them to be like this :
          +

          { "src": "images/touch/icon-128x128.png", + "type": "image/png", + "sizes": "128x128" + }

          ******/
          + // ?Is it worth looking at file extentions?
          + return

          {"src":icon.src, "type":"image/png", + "sizes":(icon.width + "x" + icon.height)}

          ;
          + });
          + manifestJson.icons = manifestIcons;
          +
          + // orientation
          + // <preference name="Orientation" value="landscape" />
          + var oriPref = this.config.getGlobalPreference('Orientation');
          + if(oriPref && ["landscape","portrait"].indexOf(oriPref) > -1)

          { + manifestJson.orientation = oriPref; + }

          — End diff –

          if `Orientation` is set to `default` we should probably set the manifest value to `any`.

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/32#discussion_r122782989 — Diff: bin/template/cordova/Api.js — @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () { "locations":this.locations, "root": this.root, "name": this.platform, "version": { "version" : "1.0.0" } , + "version": { "version" : "1.0.0" } , // um, todo! "projectConfig": this.config }; }; Api.prototype.prepare = function (cordovaProject,options) { // First cleanup current config and merge project's one into own var defaultConfig = path.join(this.locations.platformRootDir,'cordova', + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova', 'defaults.xml'); - var ownConfig = this.locations.configXml; - + var ownConfigPath = this.locations.configXml; var sourceCfg = cordovaProject.projectConfig; + // If defaults.xml is present, overwrite platform config.xml with it. // Otherwise save whatever is there as defaults so it can be // restored or copy project config into platform if none exists. if (fs.existsSync(defaultConfig)) { + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - } else if (fs.existsSync(ownConfig)) { - shell.cp('-f', ownConfig, defaultConfig); - } else { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + } + else if (fs.existsSync(ownConfigPath)) { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + } + else { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); } // this._munger.reapply_global_munge().save_all(); - this.config = new ConfigParser(ownConfig); + // merge our configs + this.config = new ConfigParser(ownConfigPath); xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(), this.config.doc.getroot(), this.platform, true); + this.config.doc.getroot(), + this.platform, true); this.config.write(); /* "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - } */ - // Update own www dir with project's www assets and plugins' assets and js-files this.parser.update_www(cordovaProject.locations.www); + // Copy or Create manifest.json + // todo: move this to a manifest helper module + // output path + var manifestPath = path.join(this.locations.www,'manifest.json'); + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json'); + if(fs.existsSync(srcManifestPath)) { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + } + else { + var manifestJson = { + "background_color": "#000", + "display": "standalone" + } ; + if(this.config){ + if(this.config.name()) { + manifestJson.name = this.config.name(); + } + if(this.config.shortName()) { + manifestJson.short_name = this.config.shortName(); + } + if(this.config.packageName()) { + manifestJson.version = this.config.packageName(); + } + if(this.config.description()) { + manifestJson.description = this.config.description(); + } + if(this.config.author()) { + manifestJson.author = this.config.author(); + } + // icons + var icons = this.config.getStaticResources('browser','icon'); + var manifestIcons = icons.map(function(icon) { + // given a tag like this : + // <icon src="res/ios/icon.png" width="57" height="57" density="mdpi" /> + /* configParser returns icons that look like this : + { src: 'res/ios/icon.png', + target: undefined, + density: 'mdpi', + platform: null, + width: 57, + height: 57 + } ******/ + /* manifest expects them to be like this : + { "src": "images/touch/icon-128x128.png", + "type": "image/png", + "sizes": "128x128" + } ******/ + // ?Is it worth looking at file extentions? + return {"src":icon.src, "type":"image/png", + "sizes":(icon.width + "x" + icon.height)} ; + }); + manifestJson.icons = manifestIcons; + + // orientation + // <preference name="Orientation" value="landscape" /> + var oriPref = this.config.getGlobalPreference('Orientation'); + if(oriPref && ["landscape","portrait"] .indexOf(oriPref) > -1) { + manifestJson.orientation = oriPref; + } — End diff – if `Orientation` is set to `default` we should probably set the manifest value to `any`.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/32#discussion_r122787134

          — Diff: cordova-lib/cordova.js —
          @@ -1486,6 +1486,26 @@ module.exports = {

          bootstrap: function() {

          + var cache = navigator.serviceWorker.register;
          — End diff –

          This seems like a great solution for detecting if a service worker has been registered.

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/32#discussion_r122787134 — Diff: cordova-lib/cordova.js — @@ -1486,6 +1486,26 @@ module.exports = { bootstrap: function() { + var cache = navigator.serviceWorker.register; — End diff – This seems like a great solution for detecting if a service worker has been registered.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/32#discussion_r122783962

          — Diff: bin/template/cordova/Api.js —
          @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () {
          "locations":this.locations,
          "root": this.root,
          "name": this.platform,

          • "version": { "version" : "1.0.0" }

            ,
            + "version":

            { "version" : "1.0.0" }

            , // um, todo!
            "projectConfig": this.config
            };
            };

          Api.prototype.prepare = function (cordovaProject,options) {

          // First cleanup current config and merge project's one into own

          • var defaultConfig = path.join(this.locations.platformRootDir,'cordova',
            + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova',
            'defaults.xml');
            -
          • var ownConfig = this.locations.configXml;
            -
            + var ownConfigPath = this.locations.configXml;
            var sourceCfg = cordovaProject.projectConfig;
            +
            // If defaults.xml is present, overwrite platform config.xml with it.
            // Otherwise save whatever is there as defaults so it can be
            // restored or copy project config into platform if none exists.
          • if (fs.existsSync(defaultConfig)) {
            + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - }

            else if (fs.existsSync(ownConfig))

            { - shell.cp('-f', ownConfig, defaultConfig); - }

            else

            { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + }

            + else if (fs.existsSync(ownConfigPath))

            { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + }

            + else

            { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); }
          • // this._munger.reapply_global_munge().save_all();
            -
          • this.config = new ConfigParser(ownConfig);
            + // merge our configs
            + this.config = new ConfigParser(ownConfigPath);
            xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(),
          • this.config.doc.getroot(), this.platform, true);
            + this.config.doc.getroot(),
            + this.platform, true);
            this.config.write();
          • /*
          • "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - }
          • */
            -
            // Update own www dir with project's www assets and plugins' assets and js-files
            this.parser.update_www(cordovaProject.locations.www);

          + // Copy or Create manifest.json
          + // todo: move this to a manifest helper module
          + // output path
          + var manifestPath = path.join(this.locations.www,'manifest.json');
          + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json');
          + if(fs.existsSync(srcManifestPath))

          { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + }

          + else {
          + var manifestJson =

          { + "background_color": "#000", + "display": "standalone" + }

          ;
          + if(this.config){
          + if(this.config.name())

          { + manifestJson.name = this.config.name(); + }

          + if(this.config.shortName())

          { + manifestJson.short_name = this.config.shortName(); + }

          + if(this.config.packageName())

          { + manifestJson.version = this.config.packageName(); + }

          + if(this.config.description())

          { + manifestJson.description = this.config.description(); + }

          + if(this.config.author())

          { + manifestJson.author = this.config.author(); + }

          + // icons
          + var icons = this.config.getStaticResources('browser','icon');
          + var manifestIcons = icons.map(function(icon) {
          + // given a tag like this :
          + // <icon src="res/ios/icon.png" width="57" height="57" density="mdpi" />
          + /* configParser returns icons that look like this :
          +

          { src: 'res/ios/icon.png', + target: undefined, + density: 'mdpi', + platform: null, + width: 57, + height: 57 + }

          ******/
          + /* manifest expects them to be like this :
          +

          { "src": "images/touch/icon-128x128.png", + "type": "image/png", + "sizes": "128x128" + }

          ******/
          + // ?Is it worth looking at file extentions?
          + return

          {"src":icon.src, "type":"image/png", + "sizes":(icon.width + "x" + icon.height)}

          ;
          + });
          + manifestJson.icons = manifestIcons;
          +
          + // orientation
          + // <preference name="Orientation" value="landscape" />
          + var oriPref = this.config.getGlobalPreference('Orientation');
          + if(oriPref && ["landscape","portrait"].indexOf(oriPref) > -1)

          { + manifestJson.orientation = oriPref; + }

          +
          + // get start_url
          + var contentNode = this.config.doc.find('content') || {'attrib':{'src':'index.html'}}; // sensible default
          + manifestJson.start_url = contentNode.attrib.src;
          +
          + // now we get some values from start_url page ...
          + var startUrlPath = path.join(cordovaProject.locations.www,manifestJson.start_url);
          + if(fs.existsSync(startUrlPath)) {
          + var contents = fs.readFileSync(startUrlPath, 'utf-8');
          + // matches <meta name="theme-color" content="#FF0044">
          + var themeColorRegex = /<meta(?=[^>]name="theme-color")\s[^>]*content="([^>])"/i;
          + var result = themeColorRegex.exec(contents);
          + var themeColor;
          + if(result && result.length>=2)

          { + themeColor = result[1]; + }

          + else { // see if there is a preference in config.xml
          + // <preference name="StatusBarBackgroundColor" value="#000000" />
          + themeColor = this.config.getGlobalPreference('StatusBarBackgroundColor');
          — End diff –

          Should we ignore `StatusBarBackgroundColor` if it is set to 'black', '#000', '#000000'?

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/32#discussion_r122783962 — Diff: bin/template/cordova/Api.js — @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () { "locations":this.locations, "root": this.root, "name": this.platform, "version": { "version" : "1.0.0" } , + "version": { "version" : "1.0.0" } , // um, todo! "projectConfig": this.config }; }; Api.prototype.prepare = function (cordovaProject,options) { // First cleanup current config and merge project's one into own var defaultConfig = path.join(this.locations.platformRootDir,'cordova', + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova', 'defaults.xml'); - var ownConfig = this.locations.configXml; - + var ownConfigPath = this.locations.configXml; var sourceCfg = cordovaProject.projectConfig; + // If defaults.xml is present, overwrite platform config.xml with it. // Otherwise save whatever is there as defaults so it can be // restored or copy project config into platform if none exists. if (fs.existsSync(defaultConfig)) { + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - } else if (fs.existsSync(ownConfig)) { - shell.cp('-f', ownConfig, defaultConfig); - } else { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + } + else if (fs.existsSync(ownConfigPath)) { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + } + else { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); } // this._munger.reapply_global_munge().save_all(); - this.config = new ConfigParser(ownConfig); + // merge our configs + this.config = new ConfigParser(ownConfigPath); xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(), this.config.doc.getroot(), this.platform, true); + this.config.doc.getroot(), + this.platform, true); this.config.write(); /* "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - } */ - // Update own www dir with project's www assets and plugins' assets and js-files this.parser.update_www(cordovaProject.locations.www); + // Copy or Create manifest.json + // todo: move this to a manifest helper module + // output path + var manifestPath = path.join(this.locations.www,'manifest.json'); + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json'); + if(fs.existsSync(srcManifestPath)) { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + } + else { + var manifestJson = { + "background_color": "#000", + "display": "standalone" + } ; + if(this.config){ + if(this.config.name()) { + manifestJson.name = this.config.name(); + } + if(this.config.shortName()) { + manifestJson.short_name = this.config.shortName(); + } + if(this.config.packageName()) { + manifestJson.version = this.config.packageName(); + } + if(this.config.description()) { + manifestJson.description = this.config.description(); + } + if(this.config.author()) { + manifestJson.author = this.config.author(); + } + // icons + var icons = this.config.getStaticResources('browser','icon'); + var manifestIcons = icons.map(function(icon) { + // given a tag like this : + // <icon src="res/ios/icon.png" width="57" height="57" density="mdpi" /> + /* configParser returns icons that look like this : + { src: 'res/ios/icon.png', + target: undefined, + density: 'mdpi', + platform: null, + width: 57, + height: 57 + } ******/ + /* manifest expects them to be like this : + { "src": "images/touch/icon-128x128.png", + "type": "image/png", + "sizes": "128x128" + } ******/ + // ?Is it worth looking at file extentions? + return {"src":icon.src, "type":"image/png", + "sizes":(icon.width + "x" + icon.height)} ; + }); + manifestJson.icons = manifestIcons; + + // orientation + // <preference name="Orientation" value="landscape" /> + var oriPref = this.config.getGlobalPreference('Orientation'); + if(oriPref && ["landscape","portrait"] .indexOf(oriPref) > -1) { + manifestJson.orientation = oriPref; + } + + // get start_url + var contentNode = this.config.doc.find('content') || {'attrib':{'src':'index.html'}}; // sensible default + manifestJson.start_url = contentNode.attrib.src; + + // now we get some values from start_url page ... + var startUrlPath = path.join(cordovaProject.locations.www,manifestJson.start_url); + if(fs.existsSync(startUrlPath)) { + var contents = fs.readFileSync(startUrlPath, 'utf-8'); + // matches <meta name="theme-color" content="#FF0044"> + var themeColorRegex = /<meta(?= [^>] name="theme-color")\s [^>] *content="( [^>] )"/i; + var result = themeColorRegex.exec(contents); + var themeColor; + if(result && result.length>=2) { + themeColor = result[1]; + } + else { // see if there is a preference in config.xml + // <preference name="StatusBarBackgroundColor" value="#000000" /> + themeColor = this.config.getGlobalPreference('StatusBarBackgroundColor'); — End diff – Should we ignore `StatusBarBackgroundColor` if it is set to 'black', '#000', '#000000' ?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/32#discussion_r122786300

          — Diff: bin/template/www/manifest.json —
          @@ -0,0 +1,21 @@
          +{
          + "name": "My App",
          + "short_name":"My Ap",
          + "description": "Description of your app from template",
          + "start_url": "index.html",
          + "scope":"index.html",
          + "icons": [

          { + "src": "img/logo.png", + "sizes": "192x192", + "type": "image/png" + }

          , {
          + "src": "img/splash.png",
          — End diff –

          I don't think we need the splash in the list of icons. Chrome will use the background_color, short_name and icon closest to 128dp to create a splash screen.

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/32#discussion_r122786300 — Diff: bin/template/www/manifest.json — @@ -0,0 +1,21 @@ +{ + "name": "My App", + "short_name":"My Ap", + "description": "Description of your app from template", + "start_url": "index.html", + "scope":"index.html", + "icons": [ { + "src": "img/logo.png", + "sizes": "192x192", + "type": "image/png" + } , { + "src": "img/splash.png", — End diff – I don't think we need the splash in the list of icons. Chrome will use the background_color, short_name and icon closest to 128dp to create a splash screen.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/32#discussion_r122780778

          — Diff: bin/template/cordova/Api.js —
          @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () {
          "locations":this.locations,
          "root": this.root,
          "name": this.platform,

          • "version": { "version" : "1.0.0" }

            ,
            + "version":

            { "version" : "1.0.0" }

            , // um, todo!
            "projectConfig": this.config
            };
            };

          Api.prototype.prepare = function (cordovaProject,options) {

          // First cleanup current config and merge project's one into own

          • var defaultConfig = path.join(this.locations.platformRootDir,'cordova',
            + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova',
            'defaults.xml');
            -
          • var ownConfig = this.locations.configXml;
            -
            + var ownConfigPath = this.locations.configXml;
            var sourceCfg = cordovaProject.projectConfig;
            +
            // If defaults.xml is present, overwrite platform config.xml with it.
            // Otherwise save whatever is there as defaults so it can be
            // restored or copy project config into platform if none exists.
          • if (fs.existsSync(defaultConfig)) {
            + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - }

            else if (fs.existsSync(ownConfig))

            { - shell.cp('-f', ownConfig, defaultConfig); - }

            else

            { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + }

            + else if (fs.existsSync(ownConfigPath))

            { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + }

            + else

            { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); }
          • // this._munger.reapply_global_munge().save_all();
            -
          • this.config = new ConfigParser(ownConfig);
            + // merge our configs
            + this.config = new ConfigParser(ownConfigPath);
            xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(),
          • this.config.doc.getroot(), this.platform, true);
            + this.config.doc.getroot(),
            + this.platform, true);
            this.config.write();
          • /*
          • "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - }
          • */
            -
            // Update own www dir with project's www assets and plugins' assets and js-files
            this.parser.update_www(cordovaProject.locations.www);

          + // Copy or Create manifest.json
          + // todo: move this to a manifest helper module
          + // output path
          + var manifestPath = path.join(this.locations.www,'manifest.json');
          + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json');
          + if(fs.existsSync(srcManifestPath))

          { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + }

          + else {
          + var manifestJson = {
          + "background_color": "#000",
          — End diff –

          Just wondering if `background_color` default should be white instead. Not opposed to black as it is probably close to 50/50.

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/32#discussion_r122780778 — Diff: bin/template/cordova/Api.js — @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () { "locations":this.locations, "root": this.root, "name": this.platform, "version": { "version" : "1.0.0" } , + "version": { "version" : "1.0.0" } , // um, todo! "projectConfig": this.config }; }; Api.prototype.prepare = function (cordovaProject,options) { // First cleanup current config and merge project's one into own var defaultConfig = path.join(this.locations.platformRootDir,'cordova', + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova', 'defaults.xml'); - var ownConfig = this.locations.configXml; - + var ownConfigPath = this.locations.configXml; var sourceCfg = cordovaProject.projectConfig; + // If defaults.xml is present, overwrite platform config.xml with it. // Otherwise save whatever is there as defaults so it can be // restored or copy project config into platform if none exists. if (fs.existsSync(defaultConfig)) { + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - } else if (fs.existsSync(ownConfig)) { - shell.cp('-f', ownConfig, defaultConfig); - } else { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + } + else if (fs.existsSync(ownConfigPath)) { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + } + else { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); } // this._munger.reapply_global_munge().save_all(); - this.config = new ConfigParser(ownConfig); + // merge our configs + this.config = new ConfigParser(ownConfigPath); xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(), this.config.doc.getroot(), this.platform, true); + this.config.doc.getroot(), + this.platform, true); this.config.write(); /* "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - } */ - // Update own www dir with project's www assets and plugins' assets and js-files this.parser.update_www(cordovaProject.locations.www); + // Copy or Create manifest.json + // todo: move this to a manifest helper module + // output path + var manifestPath = path.join(this.locations.www,'manifest.json'); + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json'); + if(fs.existsSync(srcManifestPath)) { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + } + else { + var manifestJson = { + "background_color": "#000", — End diff – Just wondering if `background_color` default should be white instead. Not opposed to black as it is probably close to 50/50.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/32#discussion_r122786410

          — Diff: cordova-js-src/confighelper.js —
          @@ -61,14 +61,9 @@ function readConfig(success, error) {
          }
          };

          • if ("ActiveXObject" in window) {
          • // Needed for XHR-ing via file:// protocol in IE
          • xhr = new window.ActiveXObject("MSXML2.XMLHTTP");
              • End diff –

          Hahahahahaha, I'm so old.

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/32#discussion_r122786410 — Diff: cordova-js-src/confighelper.js — @@ -61,14 +61,9 @@ function readConfig(success, error) { } }; if ("ActiveXObject" in window) { // Needed for XHR-ing via file:// protocol in IE xhr = new window.ActiveXObject("MSXML2.XMLHTTP"); End diff – Hahahahahaha, I'm so old.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/32#discussion_r122785379

          — Diff: bin/template/www/cordova-sw.js —
          @@ -0,0 +1,23 @@
          +
          +// Note, these will be updated automatically at build time
          +var CACHE_VERSION = '%CACHE_VERSION%';
          +var CACHE_LIST = ['CACHE_VALUES'];
          +
          +this.addEventListener('install', function(event) {
          + // Perform install steps
          + console.log("cordova service worker is installing.");
          + event.waitUntil(caches.open(CACHE_VERSION)
          + .then(function(cache)

          { + return cache.addAll(CACHE_LIST); + }

          ));
          +});
          +
          +this.addEventListener('activate', function(event)

          { + // Perform activate steps + console.log("cordova service worker is activated."); +}

          );
          +
          +this.addEventListener('fetch', function(event) {
          + console.log("cordova service worker : fetch : " + event.request.url);
          + event.respondWith(caches.match(event.request));
          — End diff –

          All the examples I've seen from Google program this a bit more defensively:

          ```
          event.respondWith(
          caches.match(event.request)
          .then(function(response) {
          // Cache hit - return response
          if (response)

          { return response; }

          return fetch(event.request);
          }
          )
          );
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/32#discussion_r122785379 — Diff: bin/template/www/cordova-sw.js — @@ -0,0 +1,23 @@ + +// Note, these will be updated automatically at build time +var CACHE_VERSION = '%CACHE_VERSION%'; +var CACHE_LIST = ['CACHE_VALUES'] ; + +this.addEventListener('install', function(event) { + // Perform install steps + console.log("cordova service worker is installing."); + event.waitUntil(caches.open(CACHE_VERSION) + .then(function(cache) { + return cache.addAll(CACHE_LIST); + } )); +}); + +this.addEventListener('activate', function(event) { + // Perform activate steps + console.log("cordova service worker is activated."); +} ); + +this.addEventListener('fetch', function(event) { + console.log("cordova service worker : fetch : " + event.request.url); + event.respondWith(caches.match(event.request)); — End diff – All the examples I've seen from Google program this a bit more defensively: ``` event.respondWith( caches.match(event.request) .then(function(response) { // Cache hit - return response if (response) { return response; } return fetch(event.request); } ) ); ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/32#discussion_r122782108

          — Diff: bin/template/cordova/Api.js —
          @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () {
          "locations":this.locations,
          "root": this.root,
          "name": this.platform,

          • "version": { "version" : "1.0.0" }

            ,
            + "version":

            { "version" : "1.0.0" }

            , // um, todo!
            "projectConfig": this.config
            };
            };

          Api.prototype.prepare = function (cordovaProject,options) {

          // First cleanup current config and merge project's one into own

          • var defaultConfig = path.join(this.locations.platformRootDir,'cordova',
            + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova',
            'defaults.xml');
            -
          • var ownConfig = this.locations.configXml;
            -
            + var ownConfigPath = this.locations.configXml;
            var sourceCfg = cordovaProject.projectConfig;
            +
            // If defaults.xml is present, overwrite platform config.xml with it.
            // Otherwise save whatever is there as defaults so it can be
            // restored or copy project config into platform if none exists.
          • if (fs.existsSync(defaultConfig)) {
            + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - }

            else if (fs.existsSync(ownConfig))

            { - shell.cp('-f', ownConfig, defaultConfig); - }

            else

            { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + }

            + else if (fs.existsSync(ownConfigPath))

            { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + }

            + else

            { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); }
          • // this._munger.reapply_global_munge().save_all();
            -
          • this.config = new ConfigParser(ownConfig);
            + // merge our configs
            + this.config = new ConfigParser(ownConfigPath);
            xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(),
          • this.config.doc.getroot(), this.platform, true);
            + this.config.doc.getroot(),
            + this.platform, true);
            this.config.write();
          • /*
          • "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - }
          • */
            -
            // Update own www dir with project's www assets and plugins' assets and js-files
            this.parser.update_www(cordovaProject.locations.www);

          + // Copy or Create manifest.json
          + // todo: move this to a manifest helper module
          + // output path
          + var manifestPath = path.join(this.locations.www,'manifest.json');
          + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json');
          + if(fs.existsSync(srcManifestPath))

          { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + }

          + else {
          + var manifestJson =

          { + "background_color": "#000", + "display": "standalone" + }

          ;
          + if(this.config){
          + if(this.config.name())

          { + manifestJson.name = this.config.name(); + }

          + if(this.config.shortName())

          { + manifestJson.short_name = this.config.shortName(); + }

          + if(this.config.packageName())

          { + manifestJson.version = this.config.packageName(); + }

          + if(this.config.description())

          { + manifestJson.description = this.config.description(); + }

          + if(this.config.author())

          { + manifestJson.author = this.config.author(); + }

          + // icons
          + var icons = this.config.getStaticResources('browser','icon');
          + var manifestIcons = icons.map(function(icon) {
          + // given a tag like this :
          + // <icon src="res/ios/icon.png" width="57" height="57" density="mdpi" />
          + /* configParser returns icons that look like this :
          +

          { src: 'res/ios/icon.png', + target: undefined, + density: 'mdpi', + platform: null, + width: 57, + height: 57 + }

          ******/
          + /* manifest expects them to be like this :
          +

          { "src": "images/touch/icon-128x128.png", + "type": "image/png", + "sizes": "128x128" + }

          ******/
          + // ?Is it worth looking at file extentions?
          — End diff –

          Yeah, I think it makes sense to look at the file extension to figure out the mimeType. Some folks might use jpg for their icons. In that case we should warn folks that png is the preferred format (that is based on something I remember reading from Google).

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/32#discussion_r122782108 — Diff: bin/template/cordova/Api.js — @@ -113,52 +110,135 @@ Api.prototype.getPlatformInfo = function () { "locations":this.locations, "root": this.root, "name": this.platform, "version": { "version" : "1.0.0" } , + "version": { "version" : "1.0.0" } , // um, todo! "projectConfig": this.config }; }; Api.prototype.prepare = function (cordovaProject,options) { // First cleanup current config and merge project's one into own var defaultConfig = path.join(this.locations.platformRootDir,'cordova', + var defaultConfigPath = path.join(this.locations.platformRootDir,'cordova', 'defaults.xml'); - var ownConfig = this.locations.configXml; - + var ownConfigPath = this.locations.configXml; var sourceCfg = cordovaProject.projectConfig; + // If defaults.xml is present, overwrite platform config.xml with it. // Otherwise save whatever is there as defaults so it can be // restored or copy project config into platform if none exists. if (fs.existsSync(defaultConfig)) { + if (fs.existsSync(defaultConfigPath)) { this.events.emit('verbose', 'Generating config.xml from defaults for platform "' + this.platform + '"'); - shell.cp('-f', defaultConfig, ownConfig); - } else if (fs.existsSync(ownConfig)) { - shell.cp('-f', ownConfig, defaultConfig); - } else { - shell.cp('-f', sourceCfg.path, ownConfig); + shell.cp('-f', defaultConfigPath, ownConfigPath); + } + else if (fs.existsSync(ownConfigPath)) { + this.events.emit('verbose', 'Generating defaults.xml from own config.xml for platform "' + this.platform + '"'); + shell.cp('-f', ownConfigPath, defaultConfigPath); + } + else { + this.events.emit('verbose', 'case 3"' + this.platform + '"'); + shell.cp('-f', sourceCfg.path, ownConfigPath); } // this._munger.reapply_global_munge().save_all(); - this.config = new ConfigParser(ownConfig); + // merge our configs + this.config = new ConfigParser(ownConfigPath); xmlHelpers.mergeXml(cordovaProject.projectConfig.doc.getroot(), this.config.doc.getroot(), this.platform, true); + this.config.doc.getroot(), + this.platform, true); this.config.write(); /* "browser": { - "parser_file": "../cordova/metadata/browser_parser", - "handler_file": "../plugman/platforms/browser", - "url": "https://git-wip-us.apache.org/repos/asf?p=cordova-browser.git", - "version": "~4.1.0", - "deprecated": false - } */ - // Update own www dir with project's www assets and plugins' assets and js-files this.parser.update_www(cordovaProject.locations.www); + // Copy or Create manifest.json + // todo: move this to a manifest helper module + // output path + var manifestPath = path.join(this.locations.www,'manifest.json'); + var srcManifestPath =path.join(cordovaProject.locations.www,'manifest.json'); + if(fs.existsSync(srcManifestPath)) { + // just blindly copy it to our output/www + // todo: validate it? ensure all properties we expect exist? + this.events.emit('verbose','copying ' + srcManifestPath + ' => ' + manifestPath); + shell.cp('-f',srcManifestPath,manifestPath); + } + else { + var manifestJson = { + "background_color": "#000", + "display": "standalone" + } ; + if(this.config){ + if(this.config.name()) { + manifestJson.name = this.config.name(); + } + if(this.config.shortName()) { + manifestJson.short_name = this.config.shortName(); + } + if(this.config.packageName()) { + manifestJson.version = this.config.packageName(); + } + if(this.config.description()) { + manifestJson.description = this.config.description(); + } + if(this.config.author()) { + manifestJson.author = this.config.author(); + } + // icons + var icons = this.config.getStaticResources('browser','icon'); + var manifestIcons = icons.map(function(icon) { + // given a tag like this : + // <icon src="res/ios/icon.png" width="57" height="57" density="mdpi" /> + /* configParser returns icons that look like this : + { src: 'res/ios/icon.png', + target: undefined, + density: 'mdpi', + platform: null, + width: 57, + height: 57 + } ******/ + /* manifest expects them to be like this : + { "src": "images/touch/icon-128x128.png", + "type": "image/png", + "sizes": "128x128" + } ******/ + // ?Is it worth looking at file extentions? — End diff – Yeah, I think it makes sense to look at the file extension to figure out the mimeType. Some folks might use jpg for their icons. In that case we should warn folks that png is the preferred format (that is based on something I remember reading from Google).
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/cordova-browser/pull/32

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/cordova-browser/pull/32
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/cordova-browser/pull/30

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/cordova-browser/pull/30
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/30#discussion_r119261228

          — Diff: bin/template/cordova/Api.js —
          @@ -96,6 +96,42 @@ Api.createPlatform = function (dest, config, options, events)

          { events.emit('error','createPlatform is not callable from the browser project API.'); throw(e); }

          +
          + // Create manifest.json
          + var manifestJson;
          + var manifestJsonPath = path.join(dest,'manifest.json');
          +
          + // Check if path exists and require manifestJsonPath.
          + if(fs.existsSync(manifestJsonPath)) {
          + try

          { + manifestJson = require(manifestJsonPath); + }


          + catch (e)

          { + console.log("error : " + e); + events.emit('error', 'unable to require manifest.json path.'); + }

          + } else if (manifestJson === undefined) {
          + manifestJson = {};
          + if(config){
          + if(config.name())

          { + manifestJson.name = config.name(); + }

          + if(config.shortName())

          { + manifestJson.short_name = config.shortName(); + }

          + if(config.packageName())

          { + manifestJson.version = config.packageName(); + }

          + if(config.description())

          { + manifestJson.description = config.description(); + }

          + if(config.author())

          { + manifestJson.author = config.author(); + }

          + }
          — End diff –

          Also, it may make sense to pull `manifest.theme_color` from `<preference name="StatusBarBackgroundColor" value="#000000" />`

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/30#discussion_r119261228 — Diff: bin/template/cordova/Api.js — @@ -96,6 +96,42 @@ Api.createPlatform = function (dest, config, options, events) { events.emit('error','createPlatform is not callable from the browser project API.'); throw(e); } + + // Create manifest.json + var manifestJson; + var manifestJsonPath = path.join(dest,'manifest.json'); + + // Check if path exists and require manifestJsonPath. + if(fs.existsSync(manifestJsonPath)) { + try { + manifestJson = require(manifestJsonPath); + } + catch (e) { + console.log("error : " + e); + events.emit('error', 'unable to require manifest.json path.'); + } + } else if (manifestJson === undefined) { + manifestJson = {}; + if(config){ + if(config.name()) { + manifestJson.name = config.name(); + } + if(config.shortName()) { + manifestJson.short_name = config.shortName(); + } + if(config.packageName()) { + manifestJson.version = config.packageName(); + } + if(config.description()) { + manifestJson.description = config.description(); + } + if(config.author()) { + manifestJson.author = config.author(); + } + } — End diff – Also, it may make sense to pull `manifest.theme_color` from `<preference name="StatusBarBackgroundColor" value="#000000" />`
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/30#discussion_r119261009

          — Diff: bin/template/cordova/Api.js —
          @@ -96,6 +96,42 @@ Api.createPlatform = function (dest, config, options, events)

          { events.emit('error','createPlatform is not callable from the browser project API.'); throw(e); }

          +
          + // Create manifest.json
          + var manifestJson;
          + var manifestJsonPath = path.join(dest,'manifest.json');
          +
          + // Check if path exists and require manifestJsonPath.
          + if(fs.existsSync(manifestJsonPath)) {
          + try

          { + manifestJson = require(manifestJsonPath); + }


          + catch (e)

          { + console.log("error : " + e); + events.emit('error', 'unable to require manifest.json path.'); + }

          + } else if (manifestJson === undefined) {
          + manifestJson = {};
          + if(config){
          + if(config.name())

          { + manifestJson.name = config.name(); + }

          + if(config.shortName())

          { + manifestJson.short_name = config.shortName(); + }

          + if(config.packageName())

          { + manifestJson.version = config.packageName(); + }

          + if(config.description())

          { + manifestJson.description = config.description(); + }

          + if(config.author())

          { + manifestJson.author = config.author(); + }

          + }
          — End diff –

          For some reason, my review lost comments. I mentioned that we should add `manifest.start_url` which can be pulled from the `<content src="index.html" />` tag in config.xml

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/30#discussion_r119261009 — Diff: bin/template/cordova/Api.js — @@ -96,6 +96,42 @@ Api.createPlatform = function (dest, config, options, events) { events.emit('error','createPlatform is not callable from the browser project API.'); throw(e); } + + // Create manifest.json + var manifestJson; + var manifestJsonPath = path.join(dest,'manifest.json'); + + // Check if path exists and require manifestJsonPath. + if(fs.existsSync(manifestJsonPath)) { + try { + manifestJson = require(manifestJsonPath); + } + catch (e) { + console.log("error : " + e); + events.emit('error', 'unable to require manifest.json path.'); + } + } else if (manifestJson === undefined) { + manifestJson = {}; + if(config){ + if(config.name()) { + manifestJson.name = config.name(); + } + if(config.shortName()) { + manifestJson.short_name = config.shortName(); + } + if(config.packageName()) { + manifestJson.version = config.packageName(); + } + if(config.description()) { + manifestJson.description = config.description(); + } + if(config.author()) { + manifestJson.author = config.author(); + } + } — End diff – For some reason, my review lost comments. I mentioned that we should add `manifest.start_url` which can be pulled from the `<content src="index.html" />` tag in config.xml
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/30#discussion_r118943490

          — Diff: bin/template/cordova/Api.js —
          @@ -96,6 +96,40 @@ Api.createPlatform = function (dest, config, options, events)

          { events.emit('error','createPlatform is not callable from the browser project API.'); throw(e); }

          +
          + // Create manifest.json
          + var manifestJson;
          + var manifestJsonPath = path.join(dest,'manifest.json');
          +
          + // Check if path exists and require manifestJsonPath.
          + if(fs.existsSync(manifestJsonPath)) {
          + try

          { + manifestJson = require(manifestJsonPath); + }


          + catch (e)

          { + console.log("error : " + e); + events.emit('error', 'unable to require manifest.json path.'); + }

          + } else if (manifestJson === undefined) {
          + manifestJson = {};
          + if(config){
          + if(config.name())

          { + manifestJson.name = config.name(); + }

          + if(config.packageName())

          { + manifestJson.version = config.packageName(); + }

          + if(config.description())

          { + manifestJson.description = config.description(); + }

          +
          + if(config.author())

          { + manifestJson.author = config.author(); + }

          + }
          — End diff –

          Should also add start url.

          ```
          manifestJson.start_url = config.content();
          ```

          I know the helper method content doesn't exist but getting the value of the src attribute of the content tag should be pretty easy.

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/30#discussion_r118943490 — Diff: bin/template/cordova/Api.js — @@ -96,6 +96,40 @@ Api.createPlatform = function (dest, config, options, events) { events.emit('error','createPlatform is not callable from the browser project API.'); throw(e); } + + // Create manifest.json + var manifestJson; + var manifestJsonPath = path.join(dest,'manifest.json'); + + // Check if path exists and require manifestJsonPath. + if(fs.existsSync(manifestJsonPath)) { + try { + manifestJson = require(manifestJsonPath); + } + catch (e) { + console.log("error : " + e); + events.emit('error', 'unable to require manifest.json path.'); + } + } else if (manifestJson === undefined) { + manifestJson = {}; + if(config){ + if(config.name()) { + manifestJson.name = config.name(); + } + if(config.packageName()) { + manifestJson.version = config.packageName(); + } + if(config.description()) { + manifestJson.description = config.description(); + } + + if(config.author()) { + manifestJson.author = config.author(); + } + } — End diff – Should also add start url. ``` manifestJson.start_url = config.content(); ``` I know the helper method content doesn't exist but getting the value of the src attribute of the content tag should be pretty easy.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on a diff in the pull request:

          https://github.com/apache/cordova-browser/pull/30#discussion_r118941014

          — Diff: bin/template/cordova/Api.js —
          @@ -96,6 +96,40 @@ Api.createPlatform = function (dest, config, options, events)

          { events.emit('error','createPlatform is not callable from the browser project API.'); throw(e); }

          +
          + // Create manifest.json
          + var manifestJson;
          + var manifestJsonPath = path.join(dest,'manifest.json');
          +
          + // Check if path exists and require manifestJsonPath.
          + if(fs.existsSync(manifestJsonPath)) {
          + try

          { + manifestJson = require(manifestJsonPath); + }


          + catch (e)

          { + console.log("error : " + e); + events.emit('error', 'unable to require manifest.json path.'); + }

          + } else if (manifestJson === undefined) {
          + manifestJson = {};
          + if(config){
          + if(config.name())

          { + manifestJson.name = config.name(); + }

          — End diff –

          We should add short name as well.

          ```
          manifestJson.short_name = config.shortName();
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/30#discussion_r118941014 — Diff: bin/template/cordova/Api.js — @@ -96,6 +96,40 @@ Api.createPlatform = function (dest, config, options, events) { events.emit('error','createPlatform is not callable from the browser project API.'); throw(e); } + + // Create manifest.json + var manifestJson; + var manifestJsonPath = path.join(dest,'manifest.json'); + + // Check if path exists and require manifestJsonPath. + if(fs.existsSync(manifestJsonPath)) { + try { + manifestJson = require(manifestJsonPath); + } + catch (e) { + console.log("error : " + e); + events.emit('error', 'unable to require manifest.json path.'); + } + } else if (manifestJson === undefined) { + manifestJson = {}; + if(config){ + if(config.name()) { + manifestJson.name = config.name(); + } — End diff – We should add short name as well. ``` manifestJson.short_name = config.shortName(); ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on the issue:

          https://github.com/apache/cordova-browser/pull/30

          @audreyso looks good. Is there a follow up issue to read the `icon` tag from config.xml and add it to the manifest?

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on the issue: https://github.com/apache/cordova-browser/pull/30 @audreyso looks good. Is there a follow up issue to read the `icon` tag from config.xml and add it to the manifest?
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user audreyso opened a pull request:

          https://github.com/apache/cordova-browser/pull/30

          CB-12804 : manifest.json added to browser during create

          <!--
          Please make sure the checklist boxes are all checked before submitting the PR. The checklist
          is intended as a quick reference, for complete details please see our Contributor Guidelines:

          http://cordova.apache.org/contribute/contribute_guidelines.html

          Thanks!
          -->

              1. Platforms affected
              1. What does this PR do?
                manifest.json added to browser during create
              1. What testing has been done on this change?
              1. Checklist
          • [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
          • [X] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
          • [X] Added automated test coverage as appropriate for this change.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/audreyso/cordova-browser CB-12804

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/cordova-browser/pull/30.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #30


          commit b63e9416fe7a1533f637987c3048994455059a37
          Author: Audrey So <audreyso@apache.org>
          Date: 2017-05-26T16:38:09Z

          CB-12804 : manifest.json added to browser during create


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user audreyso opened a pull request: https://github.com/apache/cordova-browser/pull/30 CB-12804 : manifest.json added to browser during create <!-- Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines: http://cordova.apache.org/contribute/contribute_guidelines.html Thanks! --> Platforms affected What does this PR do? manifest.json added to browser during create What testing has been done on this change? Checklist [X] [Reported an issue] ( http://cordova.apache.org/contribute/issues.html ) in the JIRA database [X] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected. [X] Added automated test coverage as appropriate for this change. You can merge this pull request into a Git repository by running: $ git pull https://github.com/audreyso/cordova-browser CB-12804 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-browser/pull/30.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #30 commit b63e9416fe7a1533f637987c3048994455059a37 Author: Audrey So <audreyso@apache.org> Date: 2017-05-26T16:38:09Z CB-12804 : manifest.json added to browser during create

            People

            • Assignee:
              auso Audrey So
              Reporter:
              auso Audrey So
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development