Last Comment Bug 238898 - Error Console evaluates commands only one time
: Error Console evaluates commands only one time
Status: VERIFIED FIXED
:
Product: Toolkit Graveyard
Classification: Graveyard
Component: Error Console (show other bugs)
: Trunk
: All All
-- minor (vote)
: mozilla1.9alpha5
Assigned To: Simon Bünzli
:
:
Mentors:
: 292107 316994 326182 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-03-27 06:42 PST by Pau Garcia i Quiles
Modified: 2016-06-29 11:02 PDT (History)
11 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
always reset the iframe (2.97 KB, patch)
2006-07-14 03:02 PDT, Simon Bünzli
neil: superreview-
Details | Diff | Splinter Review
use contentDocument.location (2.81 KB, patch)
2006-08-13 12:46 PDT, Simon Bünzli
no flags Details | Diff | Splinter Review
use Evaluator.location (2.80 KB, patch)
2006-08-13 13:11 PDT, Simon Bünzli
neil: superreview+
Details | Diff | Splinter Review
removed blank.html (and fixed bug 369097) (5.89 KB, patch)
2007-05-11 08:57 PDT, Simon Bünzli
gavin.sharp: review-
Details | Diff | Splinter Review
fixed breakage (8.59 KB, patch)
2007-05-11 09:30 PDT, Simon Bünzli
no flags Details | Diff | Splinter Review
without the patch for bug 369097 (8.59 KB, patch)
2007-05-11 09:33 PDT, Simon Bünzli
gavin.sharp: review+
Details | Diff | Splinter Review

Description User image Pau Garcia i Quiles 2004-03-27 06:42:33 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040206 Firefox/0.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040206 Firefox/0.8

If you write a Javascript expression in the Javascript console, then clear the
output in the JS Console and then try to evaluate the same expression, it
doesn't work.

Reproducible: Always
Steps to Reproduce:
1. Go to the Javascript console
2. Write a Javascript expression
3. Hit Enter or clic Evaluate
(it works)
4. Clic the Clean button, but do not erase or modify the Javascript expression
5. Hit Enter or clic Evaluate
Actual Results:  
Firefox doesn't evaluate the Javascript expression until you alter it. If you
just delete some character(s) and then write again the same characters in the
same position, it still won't evaluate the Javascript expression. You need to
add or remove characters.

Expected Results:  
The Javascript expression should have been evaluated without needing to alter it
Comment 1 User image Henrik Skupin (:whimboo) [away 02/18 - 02/27] 2004-03-27 16:40:48 PST
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b)
Gecko/20040327 Firefox/0.8.0+

It's enough to type a command like "alert('');" and hitting Enter one time. This
alert box is displayed. All further evaluations doesn't open the alert box -
nothing happens.

-> QA
Comment 2 User image José Jeria 2004-03-28 04:44:35 PST
Does this happend in Mozilla suite as well?
Comment 3 User image Henrik Skupin (:whimboo) [away 02/18 - 02/27] 2004-03-28 04:58:59 PST
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b)
Gecko/20040316

-> Javascript Console
Comment 4 User image JongAm Park 2005-04-28 06:32:21 PDT
*** Bug 292107 has been marked as a duplicate of this bug. ***
Comment 5 User image Jerry Baker 2005-11-18 08:04:25 PST
*** Bug 316994 has been marked as a duplicate of this bug. ***
Comment 6 User image Jesse Ruderman 2006-02-06 22:38:32 PST
*** Bug 326182 has been marked as a duplicate of this bug. ***
Comment 7 User image Simon Bünzli 2006-07-14 03:02:38 PDT
Created attachment 229229 [details] [diff] [review]
always reset the iframe

This was almost fixed by bug 328932 already.
Comment 8 User image neil@parkwaycc.co.uk 2006-07-21 04:48:55 PDT
bz, is the result of multiple setting the src of a frame even deterministic?
Comment 9 User image Boris Zbarsky [:bz] (still a bit busy) 2006-07-21 12:51:10 PDT
Yes.  The second set will generally cancel the load of the first set, unless the second set is a javascript: URI which produces no output.
Comment 10 User image neil@parkwaycc.co.uk 2006-08-12 14:28:13 PDT
Comment on attachment 229229 [details] [diff] [review]
always reset the iframe

bz says that the right way to force a frame to reload is to set its location rather than muck about with src attributes. (Make sure you don't loop infinitely.)
Comment 11 User image Simon Bünzli 2006-08-13 12:46:16 PDT
Created attachment 233499 [details] [diff] [review]
use contentDocument.location

This works for me and doesn't require resetting the iframe to about:blank at all.
Comment 12 User image neil@parkwaycc.co.uk 2006-08-13 13:03:00 PDT
Comment on attachment 229229 [details] [diff] [review]
always reset the iframe

Whoops, typo ;-)
Comment 13 User image neil@parkwaycc.co.uk 2006-08-13 13:06:07 PDT
Comment on attachment 233499 [details] [diff] [review]
use contentDocument.location

Won't this regress bug 158475?

>+  iframe.contentDocument.location = "javascript: " + code;
Nit: Evaluator.location should work here.
Comment 14 User image Simon Bünzli 2006-08-13 13:11:46 PDT
Created attachment 233505 [details] [diff] [review]
use Evaluator.location
Comment 15 User image neil@parkwaycc.co.uk 2006-08-17 02:00:29 PDT
Comment on attachment 233505 [details] [diff] [review]
use Evaluator.location

I was sure I had reproduced bug 158475 earlier this week but it was getting late so I couldn't debug it at the time. Then yesterday I couldn't reproduce it, so I guess the patch is fine after all. Sorry for the delay.
Comment 16 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-05-10 12:07:15 PDT
Comment on attachment 233505 [details] [diff] [review]
use Evaluator.location

I guess I have two questions about this patch:
A) Can you also remove blank.html now? Seems like removing the only remaining reference after applying this patch (attribute on the iframe in console.xul) doesn't have any negative effects, though I'm not sure why it doesn't regress bug 328932.
B) Looks like it relies on the global scope polluter (or something equally magical) to reference Evaluator directly, is that really desirable?
Comment 17 User image Simon Bünzli 2007-05-10 12:59:53 PDT
(In reply to comment #16)
> A) Can you also remove blank.html now?

There's one further instance (which should keep bug 328932 from regressing):
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/console/content/console.xul&rev=1.11&mark=132#129

> B) Looks like it relies on the global scope polluter

See comment #13 and the first line of function displayResult. Seems like reason for a different bug should we want to change this.
Comment 18 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-05-10 13:46:41 PDT
(In reply to comment #17)
> (In reply to comment #16)
> > A) Can you also remove blank.html now?
> 
> There's one further instance (which should keep bug 328932 from regressing):
> http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/console/content/console.xul&rev=1.11&mark=132#129

That's what I meant by "the only remaining reference", removing it doesn't regress that bug. Neil has theorized that that other bug was fixed on the trunk by bz's security architecture work, so I think you should just remove that reference and the file itself (from xpfe too).
Comment 19 User image Simon Bünzli 2007-05-11 08:57:29 PDT
Created attachment 264477 [details] [diff] [review]
removed blank.html (and fixed bug 369097)

(In reply to comment #18)
> That's what I meant by "the only remaining reference"

I misread. This patch now additionally removes all references to blank.html and - while I'm at it - also fixes bug 369097 for both Toolkit and XPFE (which apparently didn't get the fix for bug 342662 to begin with).
Comment 20 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-05-11 09:12:44 PDT
Comment on attachment 264477 [details] [diff] [review]
removed blank.html (and fixed bug 369097)

>Index: toolkit/components/console/content/console.js

> function evaluateTypein()
> {
>   var code = gTextBoxEval.value;
>   var iframe = document.getElementById("Evaluator");
>-  iframe.setAttribute("src", "javascript: " + encodeURIComponent(code));
>+  iframe.location = "javascript: " + code.replace(/%/g, "%25);

Missing the closing quote here. You also need to use iframe.contentDocument.location if you're going to take this approach. This change should be applied to both toolkit and xpfe versions, to keep them in sync (Neil said he was fine with this on IRC).

You also need to remove the blank.html entries from jar.mn.

With those changes, I'll r+.
Comment 21 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-05-11 09:15:40 PDT
Oh, and I was wanting to patch that escaping bug too, but I think it's best to leave that to a separate patch in the other bug (I can r+ quickly and land it for you).
Comment 22 User image Simon Bünzli 2007-05-11 09:30:15 PDT
Created attachment 264482 [details] [diff] [review]
fixed breakage

(In reply to comment #20)
> Missing the closing quote here.

Good catch.

> iframe.contentDocument.location [...] both toolkit and xpfe versions

Done.

> You also need to remove the blank.html entries from jar.mn.

Done.
Comment 23 User image Simon Bünzli 2007-05-11 09:33:33 PDT
Created attachment 264483 [details] [diff] [review]
without the patch for bug 369097

(In reply to comment #21)
> leave that to a separate patch

Missed that one. Now done.
Comment 24 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-05-11 09:55:10 PDT
Comment on attachment 264483 [details] [diff] [review]
without the patch for bug 369097

Hrm, this doesn't behave properly with javascript: URLs that don't cause a new load, e.g. evaluating ("const a=1;", "a;", "a;") will result in ("", 1, error) when it should result in ("", error, error), but I guess that happens currently too because displayResult() is only called onload. This was mentioned in bug 158475 comment 4, so I guess it's known. Might be worth filing a bug on it anyways.
Comment 25 User image Simon Bünzli 2007-05-11 10:51:46 PDT
(In reply to comment #24)
Filed bug 380422 for this issue.
Comment 26 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-05-11 11:34:45 PDT
mozilla/toolkit/components/console/jar.mn 	1.7
mozilla/toolkit/components/console/content/console.js 	1.7
mozilla/toolkit/components/console/content/console.xul 	1.12
mozilla/toolkit/components/console/content/blank.html removed
mozilla/xpfe/components/console/resources/content/console.js 	1.26
mozilla/xpfe/components/console/resources/content/console.xul 	1.38
mozilla/xpfe/components/console/resources/content/blank.html removed
Comment 27 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-05-11 11:36:38 PDT
Oh, and mozilla/xpfe/components/jar.mn 	1.90, too (oops).
Comment 28 User image Henrik Skupin (:whimboo) [away 02/18 - 02/27] 2007-05-11 16:14:19 PDT
v. with CVS build of Thunderbird version 3.0a1 (20070511)

Note You need to log in before you can comment on or make changes to this bug.