This is a regression from bug 865947. Filing as security-sensitive just in case. marquee's _setEventListener does not work because new XPCNativeWrapper.unwrap(window).Function("event", aValue); causes the following error: TypeError: XPCNativeWrapper.unwrap is not a constructor

Are we about to ship this web compat regression in 21? :( Wish we had test coverage for marquee...

Marking as sec-other because there's no known problem beyond breaking marquee. Please feel free to change to high or whatever as appropriate.

Security concerns are secondary, so renominating. Can somebody go into specifics about the user impact here?

The user impact is that scripts manipulating marquee will be broken on some sites. In practice, I believe this is mostly a problem in China; marquee is pretty much unused elsewhere.

So, at first I had a mind to write a bunch of extensive test coverage for this stuff, and converge with the spec. But as I started digging into the various quirks of our implementation, I started to get concerned about all the ominous IE6 compat comments, and realized that this isn't the most worthwhile battle for me to fight right now. So I'll just fix this bug. In fixing this I also ran into bug 872772. I'm fixing that separately, but given that this is going to need to be backported, I'm going to trivially work around it in this bug.

Comment on attachment 750202 [details] [diff] [review] Fix marquee _setEventListener. v1 r=me

(In reply to Boris Zbarsky (:bz) from comment #4) > The user impact is that scripts manipulating marquee will be broken on some > sites. Please let us know if there are any known websites that may be broken. > > In practice, I believe this is mostly a problem in China; marquee is pretty > much unused elsewhere. Based on above, I am ccing Hong Tang from Moz China to see if they have seen/heard of any web-compat regressions in support forums or via bug reports.

I am tracking this as this is a web-compat regression.At this point I don't think this is a driver for 21.0.1 or a ride-along even(to avoid possibility of new fallouts). I want to get more information on how many real world websites may be impacted and will "wontfix" accordingly for Fx21 or see what the next best step may be. When the patch is ready we should already be uplifting this to other branches with clear risk evaluation & testing needed or completed.

Comment on attachment 750202 [details] [diff] [review] Fix marquee _setEventListener. v1 [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 865947 User impact if declined: <marquee onfoo=""> listeners are broken Testing completed: Just pushed to m-i. Risk to taking this patch (and alternatives if risky): Not risky. The lines it changes are currently just straight up broken (that is to say, they just throw) in the builds we ship. We would have noticed if we had any test coverage of marquee at all. String or UUID changes made by this patch: None

Comment on attachment 750202 [details] [diff] [review] Fix marquee _setEventListener. v1 Approving for all affected branches. Is FF17/18 really affected?

(In reply to Alex Keybl [:akeybl] from comment #13) > Comment on attachment 750202 [details] [diff] [review] > Fix marquee _setEventListener. v1 > > Approving for all affected branches. Is FF17/18 really affected? They are not, because we didn't land bug 865947 there. Sorry for the confusion.

It seems that there is a compat regression. When onfoo handler is called, |this| should refer to event.target, but |this| refers to the window on trunk.

Comment on attachment 751733 [details] [diff] [review] Addendum: Make sure |this|-binding is correct or marquee event listeners. v1 r=me. Really wish we could stop faking this stuff and just do C++ event handlers. :(

https://hg.mozilla.org/integration/mozilla-inbound/rev/41c950a2f7db (Note that this addendum should be uplifted together with the original patch)

https://hg.mozilla.org/releases/mozilla-aurora/rev/d9a77ea59125 https://hg.mozilla.org/releases/mozilla-beta/rev/0aa7d9923399 With follow-up folded in.

Verified as fixed on Mozilla/5.0 (Windows NT 5.1; rv:22.0) Gecko/20100101 Firefox/22.0 (20130528181031).

I filed bug 880425 on killing off this code completely.