Closed Bug 520572 Opened 11 years ago Closed 10 years ago

watch() on an outer object should (optionally?) set watch on inner object


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
status1.9.2 --- beta5-fixed


(Reporter: bzbarsky, Assigned: mrbkap)



(Whiteboard: [firebug-p2] fixed-in-tracemonkey)


(2 files)

See the "var, window, and" thread in

Basically, Firebug would like to watch unqualified global variable assignments, and has no way to do so right now.
Honza, until we get this we need to disable direct properties of 'window' level in the Break on Property Change. Otherwise user changes like
  x = 5;
will fail to hit and look like a bug.
Whiteboard: [firebug-p2]
Flags: blocking1.9.2?
Done (Firebug SVN, R4493)
um, that svn link points to mozilla's svn repo.

any progress on this? Still requested for blocking.
OS: Mac OS X → All
Hardware: x86 → All
Could someone add STR here?
Also, does it happen on the 192 branch? This bug is listed as a Trunk defect.
str:"x", function(propname, oldval, newval) { alert('x set to ' + newval); }); var x = 5; x = 7; window.x = 8; void(0)

Should give three alerts; only gives 1.

And this should be a problem on all branches that have splitwindow (1.8.0 and later for sure).
Attached patch FixSplinter Review
This needs a test (it should be possible to test this if any of our harnesses can pass -z to the shell before running it. Here's a shell session that I used to test:

$ dbg-obj/js -z
js>'x', function() { print("HERE") } )
js> this.x = 4
js> x = 6
Assignee: general → mrbkap
Attachment #410283 - Flags: review?(brendan)
Comment on attachment 410283 [details] [diff] [review]

>+    /*
>+     * If, by unwrapping and innerizing, we changed the object check again to

Nit: comma before "check again".

Attachment #410283 - Flags: review?(brendan) → review+
Flags: blocking1.9.2? → blocking1.9.2+
I apparently checked this in a little bit ago... I'm working on the test right now.
Whiteboard: [firebug-p2] → [firebug-p2] fixed-in-tracemonkey
Attached patch TestSplinter Review
This tests this patch. I chose a basically-random directory to put the test in. bc: do you have any thoughts about where it should go?

In order to test this, I added a new option to evalcx(): evalcx("split") returns a split object to the caller, when they can then use to test split-objecty stuff on. I've verified that a build pre-attachment 410283 [details] [diff] [review] fails the test and post-attachment 410283 [details] [diff] [review] passes it.
Attachment #412825 - Flags: review?(brendan)
Attachment #412825 - Flags: review?(bclary)
Comment on attachment 412825 [details] [diff] [review]

The location seems fine. You need to test if evalcx exists and default to pass if it does not exist since this will be executed in the browser as well. Just set expect = actual = 'skipped' with a message that the test is skipped if evalcx doesn't exist. 

r+ with that.
Attachment #412825 - Flags: review?(bclary) → review+
Attachment #412825 - Flags: review?(brendan) → review+
Comment on attachment 412825 [details] [diff] [review]

> static const JSErrorFormatString *
> my_GetErrorMessage(void *userRef, const char *locale, const uintN errorNumber);
> static JSObject *
>-split_setup(JSContext *cx);
>+split_setup(JSContext *cx, JSBool evalcx);

Pre-nit: add a blank line between declarations.

>+    if (srclen == 4) {
>+        if (src[0] == 'l' && src[1] == 'a' && src[2] == 'z' && src[3] == 'y') {
>+            lazy = JS_TRUE;
>+            srclen = 0;
>+        }
>+    } else if (srclen == 5 &&
>+               src[0] == 's' && src[1] == 'p' && src[2] == 'l' &&
>+               src[3] == 'i' && src[4] == 't') {
>+        split = lazy = JS_TRUE;
>         srclen = 0;
>     }

Prettier if you keep parallel structure (nested if in both outer then clauses).

r=me with these picked, thanks.

Depends on: 529679
Closed: 10 years ago
Resolution: --- → FIXED
These bugs landed after b4 was cut. Moving flag out.
You need to log in before you can comment on or make changes to this bug.