You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by pu...@apache.org on 2013/07/10 03:53:31 UTC

js commit: [All] patch, in case console.warn is not defined

Updated Branches:
  refs/heads/master 0ce471840 -> 984b1f1e2


[All] patch, in case console.warn is not defined


Project: http://git-wip-us.apache.org/repos/asf/cordova-js/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-js/commit/984b1f1e
Tree: http://git-wip-us.apache.org/repos/asf/cordova-js/tree/984b1f1e
Diff: http://git-wip-us.apache.org/repos/asf/cordova-js/diff/984b1f1e

Branch: refs/heads/master
Commit: 984b1f1e26b12af2ad2acda96074341853ea8706
Parents: 0ce4718
Author: Jesse MacFadyen <pu...@gmail.com>
Authored: Tue Jul 9 18:53:02 2013 -0700
Committer: Jesse MacFadyen <pu...@gmail.com>
Committed: Tue Jul 9 18:53:02 2013 -0700

----------------------------------------------------------------------
 lib/cordova.js | 7 +++++++
 1 file changed, 7 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-js/blob/984b1f1e/lib/cordova.js
----------------------------------------------------------------------
diff --git a/lib/cordova.js b/lib/cordova.js
index 2bf49ab..0a566bb 100644
--- a/lib/cordova.js
+++ b/lib/cordova.js
@@ -99,10 +99,17 @@ function createEvent(type, data) {
 }
 
 if(typeof window.console === "undefined") {
+    window.external.Notify("console was undefined, in cordova.js fixing it.");
     window.console = {
         log:function(){}
     };
 }
+// there are places in the framework where we call `warn` also, so we should make sure it exists
+if(typeof window.console.warn === "undefined") {
+    window.console.warn = function(msg) {
+        this.log("warn: " + msg);
+    }
+}
 
 var cordova = {
     define:define,


Re: js commit: [All] patch, in case console.warn is not defined

Posted by Andrew Grieve <ag...@chromium.org>.
Yeah, I see what you mean. On one hand, it's possible someone will write
console.error() as well, but otoh, that particular one hasn't come up.

It's weird too that we patch these in cordova.js instead of bootstrap.
Bootstrap is what runs first. I might have a look at moving this, but not
super important atm.


On Wed, Jul 10, 2013 at 1:01 AM, Jesse <pu...@gmail.com> wrote:

> Yeah, my mistake.  I have removed the windows only debug output.
>
> The console object does not exist, but we create an empty
> console{log:function...} in this case.  However, there are places within
> cordova.js that we console.warn, and unfortunately this happens before
> DOMContentLoaded, so it is too early for my wp bootstrap code to patch it.
>
> Changing all instances to use 'log' instead of 'warn' fixes our code,
> however there could still be cases where user code would crash before
> device ready, by calling warn, and no message would ever be shown.  Not
> sure what is the best way of patching this.
>
>
>
>
> @purplecabbage
> risingj.com
>
>
> On Tue, Jul 9, 2013 at 7:55 PM, Andrew Grieve <ag...@google.com> wrote:
>
> > Hey Jesse,
> >
> > Not sure you meant to add in the window.external.Notify into the shared
> > code. Windows specific?
> >
> > Also - why patch console.warn? We already have plugin-console, which
> > ensures all console methods are available, instead of just warn (
> >
> >
> https://git-wip-us.apache.org/repos/asf?p=cordova-plugin-console.git;a=blob;f=www/console-via-logger.js;h=4095eb3e6c98a5e0ba1fd6fed04d0183a823997d;hb=HEAD
> > ).
> >
> > Is it that warn is not available on WP? If not - maybe let's just make a
> > rule not to use .warn within CordovaJS?
> >
> >
> > On Tue, Jul 9, 2013 at 9:53 PM, <pu...@apache.org> wrote:
> >
> > > Updated Branches:
> > >   refs/heads/master 0ce471840 -> 984b1f1e2
> > >
> > >
> > > [All] patch, in case console.warn is not defined
> > >
> > >
> > > Project: http://git-wip-us.apache.org/repos/asf/cordova-js/repo
> > > Commit:
> > http://git-wip-us.apache.org/repos/asf/cordova-js/commit/984b1f1e
> > > Tree: http://git-wip-us.apache.org/repos/asf/cordova-js/tree/984b1f1e
> > > Diff: http://git-wip-us.apache.org/repos/asf/cordova-js/diff/984b1f1e
> > >
> > > Branch: refs/heads/master
> > > Commit: 984b1f1e26b12af2ad2acda96074341853ea8706
> > > Parents: 0ce4718
> > > Author: Jesse MacFadyen <pu...@gmail.com>
> > > Authored: Tue Jul 9 18:53:02 2013 -0700
> > > Committer: Jesse MacFadyen <pu...@gmail.com>
> > > Committed: Tue Jul 9 18:53:02 2013 -0700
> > >
> > > ----------------------------------------------------------------------
> > >  lib/cordova.js | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > ----------------------------------------------------------------------
> > >
> > >
> > >
> > >
> >
> http://git-wip-us.apache.org/repos/asf/cordova-js/blob/984b1f1e/lib/cordova.js
> > > ----------------------------------------------------------------------
> > > diff --git a/lib/cordova.js b/lib/cordova.js
> > > index 2bf49ab..0a566bb 100644
> > > --- a/lib/cordova.js
> > > +++ b/lib/cordova.js
> > > @@ -99,10 +99,17 @@ function createEvent(type, data) {
> > >  }
> > >
> > >  if(typeof window.console === "undefined") {
> > > +    window.external.Notify("console was undefined, in cordova.js
> fixing
> > > it.");
> > >      window.console = {
> > >          log:function(){}
> > >      };
> > >  }
> > > +// there are places in the framework where we call `warn` also, so we
> > > should make sure it exists
> > > +if(typeof window.console.warn === "undefined") {
> > > +    window.console.warn = function(msg) {
> > > +        this.log("warn: " + msg);
> > > +    }
> > > +}
> > >
> > >  var cordova = {
> > >      define:define,
> > >
> > >
> >
>

Re: js commit: [All] patch, in case console.warn is not defined

Posted by Jesse <pu...@gmail.com>.
Yeah, my mistake.  I have removed the windows only debug output.

The console object does not exist, but we create an empty
console{log:function...} in this case.  However, there are places within
cordova.js that we console.warn, and unfortunately this happens before
DOMContentLoaded, so it is too early for my wp bootstrap code to patch it.

Changing all instances to use 'log' instead of 'warn' fixes our code,
however there could still be cases where user code would crash before
device ready, by calling warn, and no message would ever be shown.  Not
sure what is the best way of patching this.




@purplecabbage
risingj.com


On Tue, Jul 9, 2013 at 7:55 PM, Andrew Grieve <ag...@google.com> wrote:

> Hey Jesse,
>
> Not sure you meant to add in the window.external.Notify into the shared
> code. Windows specific?
>
> Also - why patch console.warn? We already have plugin-console, which
> ensures all console methods are available, instead of just warn (
>
> https://git-wip-us.apache.org/repos/asf?p=cordova-plugin-console.git;a=blob;f=www/console-via-logger.js;h=4095eb3e6c98a5e0ba1fd6fed04d0183a823997d;hb=HEAD
> ).
>
> Is it that warn is not available on WP? If not - maybe let's just make a
> rule not to use .warn within CordovaJS?
>
>
> On Tue, Jul 9, 2013 at 9:53 PM, <pu...@apache.org> wrote:
>
> > Updated Branches:
> >   refs/heads/master 0ce471840 -> 984b1f1e2
> >
> >
> > [All] patch, in case console.warn is not defined
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/cordova-js/repo
> > Commit:
> http://git-wip-us.apache.org/repos/asf/cordova-js/commit/984b1f1e
> > Tree: http://git-wip-us.apache.org/repos/asf/cordova-js/tree/984b1f1e
> > Diff: http://git-wip-us.apache.org/repos/asf/cordova-js/diff/984b1f1e
> >
> > Branch: refs/heads/master
> > Commit: 984b1f1e26b12af2ad2acda96074341853ea8706
> > Parents: 0ce4718
> > Author: Jesse MacFadyen <pu...@gmail.com>
> > Authored: Tue Jul 9 18:53:02 2013 -0700
> > Committer: Jesse MacFadyen <pu...@gmail.com>
> > Committed: Tue Jul 9 18:53:02 2013 -0700
> >
> > ----------------------------------------------------------------------
> >  lib/cordova.js | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > ----------------------------------------------------------------------
> >
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/cordova-js/blob/984b1f1e/lib/cordova.js
> > ----------------------------------------------------------------------
> > diff --git a/lib/cordova.js b/lib/cordova.js
> > index 2bf49ab..0a566bb 100644
> > --- a/lib/cordova.js
> > +++ b/lib/cordova.js
> > @@ -99,10 +99,17 @@ function createEvent(type, data) {
> >  }
> >
> >  if(typeof window.console === "undefined") {
> > +    window.external.Notify("console was undefined, in cordova.js fixing
> > it.");
> >      window.console = {
> >          log:function(){}
> >      };
> >  }
> > +// there are places in the framework where we call `warn` also, so we
> > should make sure it exists
> > +if(typeof window.console.warn === "undefined") {
> > +    window.console.warn = function(msg) {
> > +        this.log("warn: " + msg);
> > +    }
> > +}
> >
> >  var cordova = {
> >      define:define,
> >
> >
>

Re: js commit: [All] patch, in case console.warn is not defined

Posted by Andrew Grieve <ag...@google.com>.
Hey Jesse,

Not sure you meant to add in the window.external.Notify into the shared
code. Windows specific?

Also - why patch console.warn? We already have plugin-console, which
ensures all console methods are available, instead of just warn (
https://git-wip-us.apache.org/repos/asf?p=cordova-plugin-console.git;a=blob;f=www/console-via-logger.js;h=4095eb3e6c98a5e0ba1fd6fed04d0183a823997d;hb=HEAD
).

Is it that warn is not available on WP? If not - maybe let's just make a
rule not to use .warn within CordovaJS?


On Tue, Jul 9, 2013 at 9:53 PM, <pu...@apache.org> wrote:

> Updated Branches:
>   refs/heads/master 0ce471840 -> 984b1f1e2
>
>
> [All] patch, in case console.warn is not defined
>
>
> Project: http://git-wip-us.apache.org/repos/asf/cordova-js/repo
> Commit: http://git-wip-us.apache.org/repos/asf/cordova-js/commit/984b1f1e
> Tree: http://git-wip-us.apache.org/repos/asf/cordova-js/tree/984b1f1e
> Diff: http://git-wip-us.apache.org/repos/asf/cordova-js/diff/984b1f1e
>
> Branch: refs/heads/master
> Commit: 984b1f1e26b12af2ad2acda96074341853ea8706
> Parents: 0ce4718
> Author: Jesse MacFadyen <pu...@gmail.com>
> Authored: Tue Jul 9 18:53:02 2013 -0700
> Committer: Jesse MacFadyen <pu...@gmail.com>
> Committed: Tue Jul 9 18:53:02 2013 -0700
>
> ----------------------------------------------------------------------
>  lib/cordova.js | 7 +++++++
>  1 file changed, 7 insertions(+)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/cordova-js/blob/984b1f1e/lib/cordova.js
> ----------------------------------------------------------------------
> diff --git a/lib/cordova.js b/lib/cordova.js
> index 2bf49ab..0a566bb 100644
> --- a/lib/cordova.js
> +++ b/lib/cordova.js
> @@ -99,10 +99,17 @@ function createEvent(type, data) {
>  }
>
>  if(typeof window.console === "undefined") {
> +    window.external.Notify("console was undefined, in cordova.js fixing
> it.");
>      window.console = {
>          log:function(){}
>      };
>  }
> +// there are places in the framework where we call `warn` also, so we
> should make sure it exists
> +if(typeof window.console.warn === "undefined") {
> +    window.console.warn = function(msg) {
> +        this.log("warn: " + msg);
> +    }
> +}
>
>  var cordova = {
>      define:define,
>
>