p5.js icon indicating copy to clipboard operation
p5.js copied to clipboard

accidental call to print() with no arguments crashes browser

Open catarak opened this issue 6 years ago • 22 comments

Nature of issue?

  • [X] Found a bug
  • [ ] Existing feature enhancement
  • [ ] New feature request

Most appropriate sub-area of p5.js?

  • [ ] Color
  • [ ] Core
  • [ ] Data
  • [ ] Events
  • [ ] Image
  • [ ] IO
  • [ ] Math
  • [ ] Typography
  • [ ] Utilities
  • [ ] WebGL
  • [X] Other (specify if possible): Environment

Which platform were you using when you encountered this?

  • [ ] Mobile/Tablet (touch devices)
  • [X] Desktop/Laptop
  • [ ] Others (specify if possible)

Details about the bug:

Posted originally in processing/p5.js-web-editor#614

Calling print(x) with an undefined variable brings up the browser print dialog. I noticed that there was a similar issue over on https://github.com/processing/p5.js-web-editor/issues/474 but that seemed to be more focused on calling print outside of setup or draw. In this case, it just happens if a variable becomes undefined.

One of my students stumbled across this by accidentally providing the dist function without enough parameters let x = dist(10, 35), which ended up returning undefined.

  • p5.js version: 0.6.0
  • Web browser and version: Chrome/Firefox/Safari
  • Operating System: MacOSX 10.12.6
  • Steps to reproduce this:Alpha editor link

catarak avatar Apr 06 '18 18:04 catarak

The P5 print is supposed to open the print dialogue, if no argument have been passed to the function. This has most likely been added to keep the ability to use the print dialogue.

core/environment.js, 41:

p5.prototype.print = function(args) {
  if (typeof args === 'undefined') {
    _windowPrint();
  } else {
    console.log.apply(console, arguments);
  }
};

It's easy to fix, just put your print() in an if statement: if(str) print(str);

ghost avatar Apr 06 '18 22:04 ghost

btw @aferriss. makes sense! gonna close this one ✨

catarak avatar Apr 06 '18 23:04 catarak

i wonder if removing the args named param and instead testing arguments.length lets us distinguish from calling print() vs print(undefined). i will investigate before we close this.

lmccart avatar Apr 07 '18 01:04 lmccart

That would work @lmccart , and it would work better...

p5.prototype.print = function() {
	if (arguments.length == 0) {
		_windowPrint();
	} else {
		console.log.apply(console, arguments);
	}
};

If we don't want to update the print() function, we should at least update the documentation since it is a feature someone implemented knowingly!

ghost avatar Apr 07 '18 10:04 ghost

I would like to re-open this issue in relationship to the web editor & it's auto-reload functionality specifically:

from the reference:

Note that calling print() without any arguments invokes the window.print() function which opens the browser's print dialog. To print a blank line to console you can write print('\n').

This functionality along with the auto sketch reload means that if you write it w/o anything inside (a common way to write functions for beginners, first the function then fill it with the parameters) you're met with a looping avalanche of print dialogs which will inevitably crash the browser.

This dual, unrelated functionality, function feels counter intuitive to me. I've had many students trip up on this in class only to lose progress in their sketch because of the print() and auto refresh in tandem.

I propose: Remove window.print() from print() and move it into it's own function classified under the output IO. perhaps as printDialog() or printCanvas()

bmoren avatar Feb 10 '19 02:02 bmoren

A problem with this approach is that print(); is a native javascript API that existing and/or non-p5 scripts may depend on.

It might be worth investigating why the web editor is behaving the way you describe. Maybe it’s running the sketch inside the print-preview panel, and the nested sketch is itself calling print() ad nauseam. If this is the cause, perhaps there is a way to disable print() when called in the preview window?

Spongman avatar Feb 10 '19 05:02 Spongman

Fixing the editor could be a good step, but with 1.0 coming up it's important to lock down syntax like this ahead of it if there is the potential for change.

There have been a number of issues opened here and in the online editor, a quick search reveals:

#3264 #2360 #1597 #1491 #887 #871

looks like the discussion on the current implementation happened here: #1579

and over on the editor: https://github.com/processing/p5.js-web-editor/issues/743 https://github.com/processing/p5.js-web-editor/issues/618 https://github.com/processing/p5.js-web-editor/issues/614 https://github.com/processing/p5.js-web-editor/issues/474

So here we have a dozen closed issues which dismiss the problem or re-direct it. We haven't solved the bug, just told people to look elsewhere or ignore it.

In my mind the confusion is that we have a function with a common name with 2 distinct functionalities which are really unrelated to one another (technically, not historically 😁).

As I said above, I think we should take care of this for beginners as I've already had to show a number of students how to force quit the browser to get out of the printer loop. And then go into talking about console.log() and what the dot means really early on.

bmoren avatar Feb 10 '19 23:02 bmoren

Personally, I think p5 should back down from overriding print at all, and prefer to rename to function or delete it in its entirety. Is it harder for a new user to learn "console.log(<>)" than it is to learn "print(<>) but you have to but something in the brackets otherwise it will try to print out the document so you have to be careful not to run it when in that state"? I wouldn't think so, when that explanation would involve essentially function overloading, even if you have to explain what the dot means in the simple case for console.log. Obviously we could just do something like printLog or consoleLog but I don't see why p5 should add an API that already exists in a relatively simple and widely used and available way. Shouldn't we prefer to teach native Javascript when we can?

That said, I have no experience of teaching people who are new, so maybe I am overstating the ease of "Just write console.log" vs "Just write print" - would be great to get some input on if anyone has decided to drop using print in teaching entirely because of this.

meiamsome avatar Feb 10 '19 23:02 meiamsome

I do think you're overstating the ease for a lot of contexts p5 is used in. At least for an understanding of the functionality, but maybe less so in the sense of just saying use this instead of that, ultimately it's just vocab that the user has to internalize. (this is what I end up doing in class) But to a 10 year old, or an 18-22yo for that matter print() is friendlier than getting into object syntax on day 1. I don't really have an issue with console.log teaching at the college level, but I'm sure some folks would be sad to lose print(). As above (#1579), there was a println() for a minute, but the discussion ended up in the functionality we have now.

bmoren avatar Feb 10 '19 23:02 bmoren

I can definitely see that there are benefits to not having the object stuff involved, and that that would be easier for new users.

I still don't really understand why print is used for a beginner anyway?

If you open your browser's log panel it's called "console". In the p5js editor it is also called "console". Nothing to me about outputting onto a console has ever screamed "print" as the word to use. It's definitely historically valid, "printf" of C which I suspect was based on the usage of teleprinters at the time, but a beginner wouldn't have any such context. I guess that to me this has always seemed a strange one, and that if I want something to output into the "console" I would expect the command to involve that name somewhere.

Looking at the history of this there is sentiment in there that is on a similar vein.

Honestly, this sounds like another situation where we need more outreach as @makeyourownalgorithmicart discussed in #3494 to decide on a path forward. Perhaps we should open this one up to a longer-form discussion? This is definitely a problem that has caused repeated issues, and that's usually a sign that something could be improved.

meiamsome avatar Feb 11 '19 01:02 meiamsome

I'm not attached to print() as the name, although I understand it's historical context and most importantly the carry over from processing. console() alone would be the most logical in my mind, although I'm sure it would cause issues with the js console object. I agree that it should be part of a bigger conversation! fwiw, in my mind, printing to the console and printing the sketch are two totally different worlds!

bmoren avatar Feb 11 '19 05:02 bmoren

My vote is to revert toprintln to both avoid this conflict and maintain compatibility with processing/arduino.

Spongman avatar Feb 11 '19 06:02 Spongman

does it make sense to re-open this issue for more community discussion?

bmoren avatar Feb 11 '19 22:02 bmoren

Is there any merit in having the editor handle this problem? If we detect that print() is being called too many times within a certain time interval we could log a warning to the user in the console to be careful with placing print inside of a function that is called every frame and prevent it from firing for x number of frames/seconds. At the moment I can't think of a situation in which someone want to open the print dialog every frame.

@catarak might have some more insight.

aferriss avatar Feb 11 '19 23:02 aferriss

may we can just disable window.print() when it's called from within the draw() function? this would still allow calling it from within event handlers. there could be an appropriate warning printed to the console when print() is called from within draw() and maybe a global variable (eg, p5.enableLegacyPrint) that re-enables the original behavior.

Spongman avatar Feb 11 '19 23:02 Spongman

I'll reopen this issue as I think with the p5 editor problem (I also had that in class just last week) it is worth reinvestigating this again. It would be nice to solve this problem in the library instead of just in the editor as the problem are essentially the same in both cases.

Personally, I think @Spongman's proposal of disabling window.print() in draw() sounds appealing to me and I think worth looking into. Renaming the function isn't that helpful I think, the console itself is a callback to text terminal which isn't something that a beginner would need to understand before logging message to the console, print() may still be fine as it have the metaphor of printing something on paper but in this case onto the console (ie the console as the paper that is printed on).

limzykenneth avatar Feb 13 '19 13:02 limzykenneth

Just had a chance to read this thread. I'm not sure if it makes sense to handle this issue in the p5.js library or in the web editor. On the one hand, this seems to be an issue with the library—you can cause the print dialog to appear infinitely without the web editor. On the other hand, it seems to come up more often because of the web editor auto-refresh option.

A thought with respect to disabling window.print() in draw()—I think there could be a lot of edge cases here. For example, if the code looked something like

function draw() {
  if (frameCount === 1000) {
    print();
  }
}

print() would be called within draw(), perhaps intentionally, but would not cause the print dialog to appear over and over again.

Another option would be to throw an error/stop the sketch if the print dialog appears too many times in time period. This could be something that could be disabled. Again, not sure if it would make sense to put this in the p5 library, or in the editor, and also don't have any implementation ideas.

catarak avatar Feb 19 '19 22:02 catarak

@catarak is this still an issue? when I try to print an undefined variable in this sketch it seems to print undefined as expected. only print() with no arguments at all opens the print dialog, as expected.

lmccart avatar Apr 09 '19 02:04 lmccart

@lmccart it has been fixed! i think it's still unclear how to handle print() within the draw() loop, and maybe a new ticket should be made for that, or this ticket should be changed to reflect that.

catarak avatar Apr 09 '19 18:04 catarak

Hm reading through all this and thinking about the open design and technical questions, might a reasonable approach just be to add a clear warning into the docs for print()?

I am also wondering if we should try to phase out the use of print() and replace it with another word. console() for example? We could support both for a while but this might lead to people being less likely to accidentally through print() into the draw.

lmccart avatar Apr 11 '19 15:04 lmccart

Just wanted to bump this thread -- the importance of not conflating the two disparate behaviors that this function exhibits is pretty important.

The frequent use of auto-reload combined with calling this function under draw() probably leads to more usability issues than what is already documented here. This is doubly true when you consider that the library is designed to be highly accessible for folks new to coding, who don't have the context, knowledge, presence of mind, or Github account to +1 this particular thread. This is triply true when you consider that many people, especially new users, may not have super powerful machines that can handle this behavior gracefully and see more browser crashes because of it.

Regarding naming choices, I agree that two obvious options present themselves:

Options window.print() function Text output function
Opt 1 print() console.log() or println()
Opt 2 printCanvas() print()

I don't think I have an informed opinion on which is more preferable, but I would venture to say that for folks new to Javascript who have some Python background (which is another language commonly used to introduce coding to new folks), the existing behavior of print() would be confusing. On the other hand, respecting existing Javascript standards for print() should be of high importance. Either way, this behavior in the draw() function is a big usability issue imo.

etcadinfinitum avatar Jul 14 '21 01:07 etcadinfinitum

I teach students using p5js, and I cannot tell you how many times one of them starts typing a print statement in their draw function with auto-refresh on and then immediately gets trapped in an infinite loop of print dialogs opening. Which makes them lose all of their unsaved code (which is usually most of it, students don't save). The print dialog is just not this important. At the very least, there should be some sort of way to break out of the print dialog loop without having to kill the tab. Personally, I recommend that anyone teaching p5js just never use the print function for printing and using console.log.

c0d3rman avatar Jan 26 '22 09:01 c0d3rman

Dear community,

I feel that this discussion has become somewhat fragmented, with two different topics mixed up together. I would like to offer my support and help to reconcile these two topics.

The two issues at hand are:

  1. The loop problem: there is a situation in which p5.js users get stuck in an infinite loop
  2. The library interface: p5.js overrides a web browser function print() which causes the problem

I propose that we separate these two issues and address them one by one, in order to find an objective solution to the problem and then reach a consensus on what the library interface should be.

1. The loop problem

The issue at hand

P5.js is a library that runs in the web browser, where the print() function means "open a dialog to print the web page". However, in Processing, which p5.js is based on, print() means "print text to the console". To accommodate both of these meanings, p5.js overrides the web browser print() function and tries to be smart by calling console.log() if there are arguments and window.print() if there are no arguments. However, if a p5.js user calls the print() function without arguments inside a draw() loop, this can cause an infinite loop, as the browser will repeatedly open and close the print dialog.

How to solve the problem

As long as we have a call to window.print() inside the p5.print() function, we create the possibility for this issue to happen. Therefore, to solve this problem we need to remove the call to window.print().

Proposed solution

In the next release of p5.js, version 1.7.0, we can:

  1. Offer a new function called windowPrint().
  2. If no parameters are provided, replace the call to window.print() by a friendly error which informs the user that no parameters were passed to print() and that if they want to print the web page, they should call windowPrint() instead.
p5.prototype.print = function(...args) {
  p5._validateParameters('print', arguments);
  console.log(...args);
};

This will result in the following user experience:

function draw() {
  print("hello world");
  // hello world

  print();
  // 🌸 p5.js says: print() was called without parameters. If you want to print the web page, call windowPrint() instead.
}

2. The library interface

The issue at hand

P5.js is a JavaScript library that is based on Processing, which is a Java library. In Java, print() is a scoped function to print text to the console. In the JavaScript (browser) world, print() is a global function to print the web page. As a result, p5.js overrides a global function and adds a conditional to call Java functionality or the original JavaScript functionality.

How to solve the problem

To address this issue, we could deprecate the existing print() interface in version 2.x. However, we could also introduce a new interface in version 1.x and start recommending it as early as possible. The name of the new interface will be up for debate, but it should be simple and easy to remember, and should not force the usage of dot notation.

I would suggest something such as output() as a replacement because it does not override the browser API. Subjectively, I think that people also relate more to print as "printing on paper".

Proposed solution

In the next minor realease of p5.js, version 1.7.0, we can:

  1. Introduce a new interface such as output() and make this the default in documentation and teaching

In the next major release of p5.js, version 2.0.0, we would:

  1. Deprecate print() which would do nothing but display friendly error message to recommend usage of new new interface instead

In the next next major release of p5.js, version 3.0.0, we would:

  1. Remove print() from the library

Another option

Another option would be to simply do nothing about it and live with the fact that we are overriding browser functions which is bad practice in general. As long as we solve the loop problem we should be ok. Though if we look long term, new major version is an opportunity to correct our mistakes.

Looking forward to your feedback on those 2 proposals 👋

Ucodia avatar Feb 22 '23 19:02 Ucodia

With something like the following and in global mode

p5.prototype.print = function(...args) {
  p5._validateParameters('print', arguments);
  console.log(...args);
};

not just the call to print() in draw() but all calls anywhere to print() would be affected. The main issue here is that it could cause more compatibility issue with other JS code/library than expected. While global mode is not great compatibility wise already with lots of pollution of the global namespace, currently there is still some guarantee that the browser's existing global API (ie. using print, console, location, open, etc) would work, which is what the case where p5's print() with no arguments delegate to window.print() is trying to preserve. Which means that if there is a third party library that a user is using that wants to call window.print() to actually print the web page, there would be no way to do this easily for that library.

window.print() might be obscure enough that the above won't be a realistic problem but just talking ideally if there is a way we can limit this overwriting behaviour to just the instances where print() is called in draw() and not elsewhere would be nice.


As for the 2.0 interface I'm not sure output() make enough intuitive sense to me as I feel the main output of my p5.js code to be the canvas and the console messages to me more secondary. But in any case we can work this out slowly since we are not really thinking about a 2.0 release in the near future for the moment.

limzykenneth avatar Mar 10 '23 11:03 limzykenneth

currently there is still some guarantee that the browser's existing global API (ie. using print, console, location, open, etc) would work, which is what the case where p5's print() with no arguments delegate to window.print() is trying to preserve. Which means that if there is a third party library that a user is using that wants to call window.print() to actually print the web page, there would be no way to do this easily for that library.

In the proposal I suggested to provide a windowPrint function which would offer a one word no dot option to actually print the page. This is already a pattern used in p5.js, for example windowWidth. Trying to guess that a user wanted to call window.print() if they provide no argument was not a good assumption which is the only reason this bug exists today.

window.print() might be obscure enough that the above won't be a realistic problem but just talking ideally if there is a way we can limit this overwriting behaviour to just the instances where print() is called in draw() and not elsewhere would be nice.

There may be a solution by looking into the function call stack (new Error().stack) but that becomes quite complex, has performance drawback and prone to browser compatibility issues compared to simply stopping to call window.print().

As for the 2.0 interface I'm not sure output() make enough intuitive sense to me as I feel the main output of my p5.js code to be the canvas and the console messages to me more secondary. But in any case we can work this out slowly since we are not really thinking about a 2.0 release in the near future for the moment.

I can agree that output may relate more to the canvas output, maybe as much as print probably means printing on paper for most people! Finding an alternative name would prove challenging and I could not think of an obvious alternative as of now.

Ucodia avatar Mar 17 '23 05:03 Ucodia

In the proposal I suggested to provide a windowPrint function which would offer a one word no dot option to actually print the page. This is already a pattern used in p5.js, for example windowWidth. Trying to guess that a user wanted to call window.print() if they provide no argument was not a good assumption which is the only reason this bug exists today.

For user written code this will work just fine, my main point is that for non-user written code, code that comes from a third party library for example, they couldn't use window.print() any more in that case and the user have no realistic way to replace those window.print() calls with windowPrint().

limzykenneth avatar Mar 17 '23 10:03 limzykenneth

For user written code this will work just fine, my main point is that for non-user written code, code that comes from a third party library for example, they couldn't use window.print() any more in that case and the user have no realistic way to replace those window.print() calls with windowPrint().

That's true, window.print() won't work as expected anymore for the intent of printing the page, though why would it not be realistic to replace window.print() with windowPrint() for this type of usage? With FES we can give a clue about the new interface if no parameters were passed to the function.

If we are against removing the call to window.print() inside print() then we are back to the second branch of the issue which is renaming the print interface to something else, though again this cannot be done in a minor version of the library as this would impact sketches that use print().

What are your thoughts and this issue? Do you think this should be fixed? Do you think it this is not an issue and the it should be closed? This discussion has been going in circles for years and I am doing my best attempt to move it to forward to a conclusion.

Ucodia avatar Mar 17 '23 16:03 Ucodia

Thank you all for working on this. I'd like to bump up this issue for the upcoming 1.7.0 release. What will be the good next steps to move this issue forward? @limzykenneth @Ucodia @davepagurek

Qianqianye avatar Jun 30 '23 00:06 Qianqianye

Is the problem with print() being in the main draw loop that it's impossible to get back to editing your sketch, since each time you cancel printing, it pops up again immediately the next frame? If so, maybe we can prompt afterwards and ask if you want to disable future prints? Something like this:

let printDisabled = false
p5.prototype.print = function(...args) {
  if (!args.length) {
    if (!printDisabled) {
      _windowPrint();
      if (window.confirm('You just tried to print the webpage. Do you want to prevent this from running again?')) {
        printDisabled = true;
      }
    }
  } else {
    console.log(...args);
  }
};

Live version here: https://editor.p5js.org/davepagurek/sketches/scaELTd-LP

This would mean that third party code that prints would get this extra popup after, which is maybe a bit annoying, but probably less annoying than getting locked out of your sketch due to a print loop. (We could maybe disable this behaviour if you disable friendly errors?)

davepagurek avatar Jun 30 '23 00:06 davepagurek

In case the method I described above works for everyone, I put up a PR for it here: https://github.com/processing/p5.js/pull/6253 Feel free to close it if we decide to go for a different solution!

davepagurek avatar Jul 05 '23 11:07 davepagurek