159849 - domthingie.on* (i.e. onclick, n' friends) is undefined and not null if no event handler is set; |with| can't be used to assign to it

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

Mozilla has roblems with statment like:

with(document.getElementById('foo'))onmouseover=function;

It looks like, Mozilla add event to layer and to document.

Comment 1

[alanjstr]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1b) Gecko/20020727

What is that example URL supposed to show?  If I move the mouse over the yellow
block, I get an event response.  What build/OS, etc?  

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

First. Move your mouse outside document window. Then open url. Then move your
mouse pointer over doc.window - First event. It's a bug!
Second. Move your mouse over yellow box - event - it's ok. Move your mouse
outside yellow box, over document - event - It's a bug.

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

Just checked on Linux - the same.

Windows 2k - moz 2002072408
Linux RedHat 7.3 - Moz 2002072204

Comment 4

[Grey Hodge (jX)]

See it too on Win98 with 1.1beta

Comment 5

[Quentin Smith]

CONFIRMED. I know some JS, and this is definitely not the way it's supposed to work.

Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.1a) Gecko/20020610
Build ID: 2002061014

Comment 6

[Grey Hodge (jX)]

I'm not sure this is a bug, as much as ambiguous code...

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: more self-explanatory testcase

Comment 8

[Boris Zbarsky [:bzbarsky]]

to JS engine.  If I understand how "with" should work correctly, "onmouseover"
and "foo" should be getting set on the div, not on the window.

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

As i know with should move root scope from window to object.

Comment 10

[Jesse Ruderman]

If this isn't a javascript engine bug, it may be an events bug.
("onmouseover" in document) is true in IE and false in Mozilla, which is why 
with(elem){onmouseover=func} adds onmouseover to window rather than elem.

Comment 11

[Phil Schwartau]

Apparently in JS you cannot use the |with| statement to DEFINE
a brand-new property of an object. That is, if |obj1| does not
already have a property called |prop|, the following statements
are not equivalent:

                   with (obj1) {prop = 99;} 
                   obj1.prop = 99;


This standalone JS shell test shows the issue:

obj1 = {};
with (obj1) {prop = 99;}
  
msg = '\n';
msg += 'this.prop: ' + this.prop + '\n';
msg += 'obj1.prop: ' + obj1.prop + '\n';
print(msg);


-------------------------  OUTPUT  ------------------------
this.prop: 99
obj1.prop: undefined



Notice the new property has been assigned to the global object
instead of |obj1|. But as Jesse points out, if |obj1| ALREADY
has a property called |prop|, then assignment can be done via
the |with| statement without any problem:

obj1 = {};
obj1.prop = 0;
with (obj1) {prop = 99;}
  
msg = '\n';
msg += 'this.prop: ' + this.prop + '\n';
msg += 'obj1.prop: ' + obj1.prop + '\n';
print(msg);


-------------------------  OUTPUT  ------------------------
this.prop: undefined
obj1.prop: 99

Comment 12

[Phil Schwartau]

From ECMA-262 Edition 3 Final:

10.1.4 Scope Chain and Identifier Resolution
 
Every execution context has associated with it a scope chain.
A scope chain is a list of objects that are searched when evaluating
an Identifier. When control enters an execution context, a scope chain
is created and populated with an initial set of objects, depending on
the type of code. During execution within an execution context, the
scope chain of the execution context is affected only by with statements
(section 12.10) and catch clauses (section 12.14). 

During execution, the syntactic production PrimaryExpression : Identifier
is evaluated using the following algorithm: 

1. Get the next object in the scope chain. If there isn't one, go to step 5. 
2. Call the [[HasProperty]] method of Result(1), passing the Identifier
   as the property. 
3. If Result(2) is true, return a value of type Reference whose base object
   is Result(1) and whose property name is the Identifier. 
4. Go to step 1. 
5. Return a value of type Reference whose base object is null and whose
   property name is the Identifier.


12.10 The with Statement 

Syntax
WithStatement : with (Expression) Statement 

Description 
The with statement adds a computed object to the front of the scope chain
of the current execution context, then executes a statement with this
augmented scope chain, then restores the scope chain. 

Semantics
The production WithStatement : with (Expression) Statement
is evaluated as follows:
 
1. Evaluate Expression. 
2. Call GetValue(Result(1)). 
3. Call ToObject(Result(2)). 
4. Add Result(3) to the front of the scope chain. 
5. Evaluate Statement using the augmented scope chain from step 4. 
6. Let C be Result(5). If an exception was thrown in step 5,
   let C be (throw, V, empty), where V is the exception. 
   (Execution now proceeds as if no exception were thrown.) 
7. Remove Result(3) from the front of the scope chain. 
8. Return C. 


NOTE
No matter how control leaves the embedded Statement, whether normally
or by some form of abrupt completion or exception, the scope chain
is always restored to its former state.

Comment 13

[Phil Schwartau]

So - when JS evaluates |with (obj1) {prop = 99;}|,
the scope chain is [obj1, global].


To resolve the identifier |prop|, it must first check the scope
of |obj1|. If we have not defined such a property on |obj1|,
it cannot find the property there, so it checks the global scope. 

It doesn't find such a property there, either! But JS is willing
to define a new property |prop| on the global object. This is the
output of the first test in Comment #11.

This is counter-intuitive to me. If JS is willing to define the
brand-new property on the global object, why isn't it willing to
define it on |obj1|?  The identifier lookup failed in both links
in the scope chain. Of course! We're trying to define the identifier!

I assume this bug is invalid, but I can't make a convincing argument.
Looking for one of the developers to explain this better than I can -

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

Phil. You're right. Boris added property just to check second thing. The main
goal is event handler "onmouseover".
Handler IS defined for element and SHOULD be used. 

I never said that with(elem)foo=1 should define elem.foo with value 1. I said
that with(elem)onmouseover=function should attach function to elem.onmouseover
rather than to window.onmouseover.

Please, focus on Boris attachment.
First we're setting value for onmouseover. Layer should has onmouseover so event 
 should be added to element.

Comment 15

[Phil Schwartau]

Zbigniew: try out this HTML in Mozilla:


<html><head><title>"onmouseover" in elem?</title></head>
<script language="JavaScript">

function main()
{
  elem = document.getElementById('div1');

  msg = ''; 
  msg += '"onmouseover" in elem ?  ' + ('onmouseover' in elem) + '<br>';
  msg += 'elem.onmouseover = ' + elem.onmouseover;

  document.write(msg);
}

</script>

<body onLoad="main();">
  <div id="div1"</div>
</body></html>



-------------------------  OUTPUT  ------------------------

               "onmouseover" in elem ? false
               elem.onmouseover = undefined


I am not a DOM expert, so I will cc jst to make sure this is 
not a bug. But it shows that after the page has loaded, the
element "div1" above does NOT have an |onmouseover| property!

As Jesse points out in Comment #10, even the document object
does not have an |onmouseover| property in the Mozilla DOM,
until you define an onmouseover handler for it.

We know we can add |onmouseover| via the dot operator, as in

                   elem.onmouseover = f;

but in Mozilla, this will be a brand-new property of the element.
Because JS does not allow brand-new properties to be created on
elements via the |with| statement, we cannot do it as
 
               with (elem) {onmouseover = f;}


As Jesse discovered, this is not the same as the IE DOM, where
the document object and div objects DO HAVE a pre-defined 
|onmouseover| property (set to the null object). 


Here is the output from IE6 on the example above:


-------------------------  OUTPUT  ------------------------

               "onmouseover" in elem ? true
               elem.onmouseover = null


So in IE, the |onmouseover| property is not brand-new, and 
so can be changed via the |with| statment from the value |null|
to a non-null value -

Comment 16

[Brendan Eich [:brendan]]

Reassign to DOM, this is an IE non-standard DOM thing, an extension.  If no top
sites use it, I don't think Mozilla should support it.

/be

Comment 17

[Phil Schwartau]

Setting default QA -

Comment 18

[Zibi Braniecki [:zbraniecki][:gandalf]]

Right. And i'm investigating if IE or Mozilla's behaviour is better.
In a way of banding routines in Mozilla to make them more compatible to IE DOM,
and if none specyfication forbid this i would add events handler as predefined.
Why?

The only change i see is in this statment with(layer)onmouseover=a; and it's
logical, that using this i want to add handler to layer rather than to window.

Comment 19

[Brendan Eich [:brendan]]

Zbigniew, please don't change the Component setting on this bug again.

/be

Comment 20

[Zibi Braniecki [:zbraniecki][:gandalf]]

Sorry, i used ctrl+r instead of shift+ctrl+r and component selector was invalid
when i sent comment.

Comment 21

[timeless]

http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onmouseover.asp
Standards Information
This event is defined in HTML 4.0 http://www.w3.org/tr/rec-html40/

Applies To
A, ADDRESS, APPLET, AREA, B, BDO, BIG, BLOCKQUOTE, BODY, BUTTON, CAPTION,
CENTER, CITE, CODE, CUSTOM, DD, DFN, DIR, DIV, DL, document, DT, EM, EMBED,
FIELDSET, FONT, FORM, hn, HR, I, IMG, INPUT type=button, INPUT type=checkbox,
INPUT type=file, INPUT type=image, INPUT type=password, INPUT type=radio, INPUT
type=reset, INPUT type=submit, INPUT type=text, KBD, LABEL, LEGEND, LI, LISTING,
MAP, MARQUEE, MENU, nextID, NOBR, OL, P, PLAINTEXT, PRE, RT, RUBY, S, SAMP,
SELECT, SMALL, SPAN, STRIKE, STRONG, SUB, SUP, TABLE, TBODY, TD, TEXTAREA,
TFOOT, TH, THEAD, TR, TT, U, UL, VAR, XMP

See Also
onmousewheel, onmousedown, onmousemove, onmouseout, onmouseup

http://www.mozilla.org/docs/dom/domref/dom_el_ref72.html
Specification
Not part of specification. 

Note that the old summary is interesting but doesn't really relate to anything.
proof:
javascript:a={};with(a){b=1;}a.b+="";[a.b,this.b]

result (mozilla, ie5):
undefined,1

since we've bounced this bug to dom based on the reporter's intent i'm munging
the summary towards something which i hope resembles the issue. I expect someone
else will have to munge it some more - sorry.

http://www.w3.org/TR/REC-html40/interact/scripts.html#adef-onmouseover
onmouseover = script [CT]
The onmouseover event occurs when the pointing device is moved onto an element.
This attribute may be used with most elements.

Offhand, i suspect this is *not* an extension issue and that we're just perhaps
broken (both in implementation and dom). but I don't speak for microsoft,
mozilla.org, standards, or the trees.

Comment 22

[Brendan Eich [:brendan]]

HTML 4.x specs do not define the DOM.

Giving this to joki and the DOM Events component is not a smart move, given that
it's not a bug in event delivery, and joki has a pile of bugs ahead of this one.
 Stop playing word-match games, think about what the component means after
reading http://bugzilla.mozilla.org/describecomponents.cgi?product=Browser.

Can we please let the DOM experts sort out component?  I'm bouncing to DOM level
0 for now.

/be

Comment 23

[timeless]

http://216.239.39.100/search?q=cache:1YsRqKGGcyYC:www.w3.org/TR/WD-DOM/level-one-html-971209.html+dom+onmouseover+onmouseout+onmousemove+onmouseup+onmousedown+htmlelement&hl=en&ie=UTF-8
there we go. dom-level-one

Comment 24

[Brendan Eich [:brendan]]

Timeless, I see no mention of those event handlers in the cited spec.  Your
google search appears over-broad.  Don't flail harder, flail smarter!

/be

Comment 25

[Johnny Stenback (:jst)]

Ok, so we should make resolving any event property (i.e. any on* property) on
any DOM thing that supports those properties actually resolve to null if there's
no event handler set in stead of leaving it undefined. That way the JS engine
would do what's expected here. Back to peterv.

Comment 26

[Johnny Stenback (:jst)]

Oh, and this is IMO not a high priority problem, we have far more serious
problems to fix unless someone can show that this breaks a lot of real sites out
there.

Comment 27

[Zibi Braniecki [:zbraniecki][:gandalf]]

No, of course it's not a big issue. I hoped it's easy to patch, and wanted to
mention the problem.

It would be nice to have it fixed as fast as possible, to make shure that most
people using Mozilla will work with with(layer){onmouseover=a;onmousout=b} and such.

I'm removing URL, because it doesnt show the problem.

Comment 28

[Daniel Wang]

-> Doron

Comment 29

[Daniel Wang]

sorry, mistake when mass assigning bugs... back to DOM

Comment 30

[Steve Chapel]

Fixing status whiteboard to be consistant with other parity-ie bugs for easier
searches. Sorry for the bugspam.

Comment 31

[Boris Zbarsky [:bzbarsky]]

*** Bug 275306 has been marked as a duplicate of this bug. ***

Comment 32

[Yuh-Ruey Chen]

Why is this blocking bug 93018?

Comment 33

[Yuh-Ruey Chen]

I meant bug 93108. sorry for bugspam

Comment 34

[Erik Fabert]

*** Bug 337966 has been marked as a duplicate of this bug. ***

Comment 35

[Boris Zbarsky [:bzbarsky]]

Attachment: This should fix it, I think

Brendan, are the flags right here?  I _think_ they are, but...  Should I toss in a JSPROP_ENUMERATE?  Or no?

Comment 36

[Nickolay_Ponomarev]

(Adding back the bit about assigning to the property using |with| to avoid confusion.)

Comment 37

[Brendan Eich [:brendan]]

(In reply to comment #35)
> Created an attachment (id=240164) [edit]
> This should fix it, I think
> 
> Brendan, are the flags right here?  I _think_ they are, but...  Should I toss
> in a JSPROP_ENUMERATE?  Or no?

To answer that, chase down what attributes are used when SetProperty binds an event handler.  You'll end up here:

http://lxr.mozilla.org/mozilla/source/dom/src/base/nsJSEnvironment.cpp#1821

and see JSPROP_ENUMERATE | JSPROP_PERMANENT.

/be

Comment 38

[Boris Zbarsky [:bzbarsky]]

Ah, ok.  Good catch.  I'll make that change before checking in.

Comment 39

[Peter Van der Beken [:peterv]]

Comment on attachment 240164 [details] [diff] [review]
This should fix it, I think

With the right flags, r=peterv.

Comment 40

[Brendan Eich [:brendan]]

Comment on attachment 240164 [details] [diff] [review]
This should fix it, I think

Sorry, was waiting for a patch with the right attributes!  sr=me with those.

/be

Comment 41

[Boris Zbarsky [:bzbarsky]]

Attachment: With the right flags and more comments

Comment 42

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 43

[Boris Zbarsky [:bzbarsky]]

Added the test to mochitest.

Comment 44

[Jesse Ruderman]

See also bug 522401, which wants the objects to actually exist, not just catch with-assignment.