369469 - XBL creating properties as events are triggered (e.g. onxblfocus) is weird

Status: RESOLVED WONTFIX

(Core :: XBL, defect)

Description

[Jesse Ruderman]

Attachment: testcase

BL adds attributes like "onxblfocus" and "onxblblur" to elements when those events are called.  I think it should either always add these attributes or never add them.

Things to notice about the testcase:
* After a call to focus(), two new properties are added.
* If the window does not have focus, only one new property is added.

I noticed this because the script in bug 326633 has behavior that depends on the exact number of properties objects have.  Because I was doing other things, the window sometimes had focus and sometimes did not, causing the script to behave chaotically.  I think I can avoid the chaotic behavior by making the script skip attributes named onxbl*, but it would have been better if I didn't have to go through the pain of discovering that.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

So this is sort of on purpose http://lxr.mozilla.org/seamonkey/source/content/xbl/src/nsXBLPrototypeHandler.cpp#408

Comment 2

[Jesse Ruderman]

I figured that much ;)  Is the purpose to cache the compiled handler?  Maybe it could be stored somewhere other than in the node's attribute list, maybe even with the XBL binding.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

There definitely are places we could store this info, but I'm not sure if it's worth it. If this is blocking testing we can definitely fix it, but otherwise it's pretty low priority for me.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Those aren't attributes; please don't scare me like that!

In any case, this is how on* stuff behaves in HTML in general, so I'd probably wontfix this.

Comment 5

[Jesse Ruderman]

Attachment: HTML testcase (no XBL)

You're right, normal HTML has similar weird behavior.  Calling focus() on a textbox that has an onfocus attribute makes both an "onfocus" property and an "addEventListener" property appear in enumeration!  If the window doesn't have focus, "addEventListener" still shows up but "onfocus" does not.

Maybe I should just make my script stay the heck away from focus/blur ;)

Comment 6

[Boris Zbarsky [:bzbarsky]]

Yeah.  You can thank JS and its name resolution business for that, fwiw.  ;)  I don't think we can do much about it short of switching to a different JS engine or predefining all possible on* properties to null up front.

I do think there's a bug on that last, btw.

Comment 7

[Jesse Ruderman]

What about the addEventListener part?

Comment 8

[Jesse Ruderman]

I won't whine too much if you wontfix it all; I'm probably the only one who cares at all about exactly what shows up in property enumeration.  Most of the pain is a sunk cost for me and I can find workarounds.

Comment 9

[Jesse Ruderman]

Bug 159849?  It's marked as fixed though.

Comment 10

[Boris Zbarsky [:bzbarsky]]

The addEventListener part is odd.  I would have thought XPConnect-exposed properties would be enumerable, and addEventListener _should_ be exposed via XPConnect (in addition to the classinfo glue that does:

6709     JSFunction *fnc =
6710       ::JS_DefineFunction(cx, obj, ::JS_GetStringBytes(str),
6711                           AddEventListenerHelper, 3, JSPROP_ENUMERATE);

to handle our extensions to it).

Comment 11

[Boris Zbarsky [:bzbarsky]]

> Bug 159849?  It's marked as fixed though.

Oh, right.  In JS, enumerating props on foo and doing |"bar" in foo| and doing "foo.bar" are kinda distinct things that can give different answers...  So for the on* props, we changed what foo.bar does, basically.  But the property doesn't actually exist till someone tries to resolve it.

Comment 12

[Jesse Ruderman]

Should I file a separate bug on the addEventListener part?

Comment 13

[Boris Zbarsky [:bzbarsky]]

I don't think it's worth it.  I'm willing to bet $20 it's "correct" given how JS works and all that...  I just happen not to know the exact codepath that leads to that result.

Comment 14

[Jesse Ruderman]

It's "correct" for addEventListener to not appear in property enumeration -- until it is used implicitly? :/

Comment 15

[Boris Zbarsky [:bzbarsky]]

Again, just my guess.  I could be wrong; jst or brendan would know for sure.  Maybe.

Comment 16

[Boris Zbarsky [:bzbarsky]]

XBL is now disabled in Firefox (Bug 1583314) and is in the process of being removed from Gecko (Bug 1566221), so closing bugs requesting changes to its implementation as wontfix.