Mozilla trunk build 2005-06-13, Win2k I have a custom XBL binding that extends one of Mozilla's mailWidgets (chrome://messenger/content/mailWidgets.xml#mail-multi-emailHeaderField), including the overloading of one of its methods (updateEmailAddressNode). This does not work on trunk anymore: when the function updateEmailAddressNode is called inside the base widget, the base widget function is called, not my replacement anymore. Maybe this is just another fallout of the automatic XPCNativeWrapper stuff? Anyway, XBL bindings that can't change the behaviour of the bindings they extend anymore are pretty useless...
This is an interesting problem: it's not clear to me at least that XBL methods should be virtual, but if they are there should be a way to expicitly call the base version (through .prototype.__parent__ perhaps?). In addition, the interactions with interface declarations is interesting: should derived bindings declare the same interfaces that base classes declare?
Summary: Can't "overload" XBL methods → XBL methods are no longer "virtual"
(In reply to comment #1) > This is an interesting problem: *g* > it's not clear to me at least that XBL methods should be virtual, Well, up to now, at least, they were... > but if they are there should be a way to expicitly call the > base version (through .prototype.__parent__ perhaps?). That's not the problem here, since I can't modify the base version. There may be some workaround by using leet JS hacking, but I don't think that this should be the way. With the help of the German Mozilla newsgroup de.comm.software.mozilla.nightly-builds, we were able to pin down the regression to a time window of 2006-06-02:15 to 2005-06-02:19. The IMO most suspect checkins there were for bug 292591 and maybe for bug 294795, but both are security bugs I can't read. I'm currently debugging that.
Karsten, could you post a bonsai link for the range you think you found? The range you list in comment 2 doesn't include the fix for bug 294795 that I can see... In any case, even if the new behavior is desirable (which I doubt) we should understand _why_ it's happening to see whether something else could also have gotten broken.
Like Karsten wrote, we have tried some Builds and cut the timeframe down to 2005060215--working and 2005060219--broken. I have give something about 10 minutes plus for ending time at the Bonsai Query, because I can't remember the Checkout-Time for the 2005060219 Build exactly: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-06-02+15%3A00%3A00&maxdate=2005-06-02+19%3A10%3A00&cvsroot=%2Fcvsroot
That range in fact doesn't include bug 294795.... In any case, what bothers me is how this could happen at all. There's only one JSObject involved, right? And the methods in question are just set on it via JS_DefineUCProperty. So as long as the more-derived binding is installed later, it should automatically "override" (rather replace) the methods from an ancestor binding. Could it be that the child binding is somehow ending up with a PR_FALSE return from AllowScripts()? That is, do methods that are _not_ replacing ancestor binding methods work?
(In reply to comment #5) > That range in fact doesn't include bug 294795.... Ah, okay, off by half an hour. > Could it be that the child binding is somehow ending up with a PR_FALSE return > from AllowScripts()? AllowScripts() gets called *a lot*. Inspired by Bonsai, I already put a bunch of printf's in, and at the time when a new element with the binding gets created, it becomes |false| at the final return, because the (successful!) call to CanExecuteScripts sets canExecute to |false|. > That is, do methods that are _not_ replacing ancestor binding methods work? No, they're missing also. The binding's <content> is there, though. I'll try build a testcase.
13 years ago
What's the filename of the file the binding is in? Do you have JS disabled, by chance? Requesting blocking on the branches that bug 292591 landed on.
Summary: XBL methods are no longer "virtual" → Some XBL bindings no longer compile the implementation when they should
What kind of XBL bindings, are they installed as chrome or are they remote?
Absent a reduced testcase which Karsten is working on, this was initially seen in Mnenhy 0.7.0.2's header display ("broken and shows just undefined for NGs etc.")
Mnenhy has XBL in .xbl files, which don't get the system principal (unlike .xml files). Since JS is off in mail windows, I would fully expect said XBL to stop working. The solutions are to either put the XBL in .xml files or fix our extension silliness for system principals in chrome somewhat.
I dug down to the point where my .xbl got a principal different from mSystemPrincipal, but didn't know how to interpret that - so I debugged myself lost in some pretty irrelevant area for the last hours... :-/ Anyway: (In reply to comment #10) > Mnenhy has XBL in .xbl files, which don't get the system principal (unlike > .xml files). Since JS is off in mail windows, I would fully expect said XBL > to stop working. That's oh so very true. :-/ > The solutions are to either put the XBL in .xml files or fix our > extension silliness for system principals in chrome somewhat. I changed the binding to use an .xml file and now it's working again. Thank you so much, bz! :) Maybe we really should at least allow .xbl for chrome URIs? So this bug here is very likely a dupe of this most probably already reported problem, but I don't find a real fit (Bug 138850 and bug 269378 are related, though)... Feel free to dupe. Sorry folks for all the hassle - who would have thought that a file suffix would stop working... :(
13 years ago
Depends on: 221490
The file suffix should NOT have stopped working, so there was no hassle. As a bare minimum, we should add "xbl" to the list of "system" extensions. Better would be to fix bug 221490.
Thanks for the detective work!
Assignee: general → dveditz
Created attachment 187009 [details] [diff] [review] branch fix for common .xbl extension only For the branches bsmedberg thought we ought to be conservative and simply make .xbl work--since that was the scope of the branch regression--rather than take on the full change in bug 221490
Whiteboard: patch in 221490 → need reviews
Comment on attachment 187009 [details] [diff] [review] branch fix for common .xbl extension only r=bsmedberg (branch only)
Attachment #187009 - Flags: review?(benjamin) → review+
Comment on attachment 187009 [details] [diff] [review] branch fix for common .xbl extension only Let's get this checked in as soon as dbaron gives the sr=. a=jay
Attachment #187009 - Flags: superreview?(dbaron) → superreview+
No longer blocking the trunk now that bug 221490 is fixed.
This fix checked into 1.7 and aviary1.0.1 branches; bug 221490 fixes this on the trunk.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed-aviary1.0.5, fixed1.7.9
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.