Closed Bug 76641 Opened 25 years ago Closed 25 years ago

nsIDOMNode API binary compatibility lost since moz0.8.1

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9

People

(Reporter: jst, Assigned: hjtoi-bugzilla)

Details

(Whiteboard: important to mozilla0.9)

Attachments

(2 files)

Revert nsIDOMNode API change, back out change to nsContextMenu.js
8.76 KB, patch
Details | Diff | Splinter Review
Complete temporary backout patch.
13.67 KB, patch
Details | Diff | Splinter Review
A few weeks ago a new attribute was added to the DOM Node interface, the attribute is 'baseURI' and is defined in the current DOM Level 3 working draft. This change broke binary backwards, and forwards (since this is only temporarily in the nsIDOMNode interface) compatibility with older mozilla milestones which could very well matter for some vendors since this is the core of all DOM node interfaces in mozilla. This API change is not really needed in order to support the new property and this property is already supported w/o API changes on the XPCDOM branch. It's been argued that this change should be temporarily backed out until the XPCDOM branch lands to avoid breaking binary compatibility. Backing this change out causes minor problems with relative XLinks (same problems we had before this change was made), but that should be the only bad side effect of taking this API change out temporarily. I'll attach a patch that reverts this API change. (PS. When the XPCDOM branch does land there will be a change in the HTMLElement interface (a higher level interface) since that interface currently contains attributes that should not be there, the branch landing will make the interface match the interfaces in the DOM Level 2 spec and the properties that currently are in that interface will be moved into a new interface. The XPCDOM branch also contain minor changes to a few other HTMLElement interfaces that were also different than the ones defined in the DOM HTML spec.)
Target Milestone: --- → mozilla0.9
transformiix relies on baseURI so please don't back it out. It would be perfectly ok if baseURI is moved into another interface as long as that interface is supported by all DOMs (XML, XUL and XHTML) and on all nodetypes.
sicking, mozilla.org does not want to ship milestone releases that break the DOM ABI, if we can help it. How much of a hardship on transformiix would it be to do without baseURI for 0.9? Cc'ing xpfe people to judge the hardship for nsContextMenu.js. /be
I would like to know if it is possible to put it in another interface, like sicking said - it's tremendously useful, and there are other places where we're going to start using it (the meta-properties dialog is the first one that comes to mind)
It should go in nsIDOM3Node, but that requires "tear-offs" in the implementations and such. I really think it's a bad meme to let out of the lab in nsIDOMNode -- people are already depending on it, making it that much harder to move to nsIDOM3Node. Can we do the right thing now, fast, while the tree is frozen? /be
Cc'ing some drivers. /be
Keywords: mozilla0.9
Whiteboard: important to mozilla0.9
Transformiix could live without baseURI (we once did). But it increases codesize and code uglyness and degrades performance. The nsIDOM3Node is a temporary interface too (at least as far as w3c's concerned) so we would still break forwards compability. We are also highly likely to run into this problem sooner or later anyway since DOM3 will most likly expand the Node interface. However I do agree that the baseURI shouldn't go in the nsIDOMNode interface just yet since it's still part of a WD and could very well change to the next WD (it has some unresolved issues)
I would assume that most of the baseURI property users want to use it via JS (like the context menu), so we would not break those no matter where the property was specified. C++ is a different thing, of course. nsIXMLContent has GetXMLBaseURI(nsIURI **aURI) method, but I am not sure if that is sufficient for XSLT. Jonas? We are trying to get rid of nsIXMLContent interface so this solution would probably also be temporary. I would be surprised if baseURI changed names or moved to some other interface than Node in the DOM WG - it is just too natural in Node to be moved. It seems a bit weird that it would be in Node IDL, but in nsIDOM3Node class. But I don't think the W3C has anything to say on this matter, it is binding specific (especially since they don't even provide a sample C++ binding). Does anyone actually know that there are some applications out there that would break because of this? I would assume that at the stage we are in Mozilla your binaries would be almost certainly incompatible between milestones (with all the API changes in strings and xpcom etc.)
nsIDOM3Node will be a permanent interface (but not frozen until the DOM Level 3 spec is done) in mozilla, just like nsIDOMNode is now. The DOM spec's might be changing interfaces, but we can't (since it changes the ABI) so we'll haveto add interfaces (which is already done on the XPCDOM branch). Performance critical code, such as transformiix should in the future not rely on using the nsIDOM3Node interface (since it's implemented using a tearoff), there will be other much more efficient ways of getting at this same information using nsIContent in the future.
Back it out if it's that important. We'll temporarily put our old code back for Transformiix, ugly as it is (or rewrite it to use a different interface if that's available).
Heikki: The reason I'm worried that the attribute might change is that there is a difference between your implementation and the xml-infoset (which only defines baseURI on elements, docuemnts and documentfragments). I for one think that your implementation is much more usefull, but not many people listen to me... ;-) I think that the best thing for Transformiix is to go back to the implementation that we used before baseURI was implemented. Patch used to remove our implementation and use mozillas is avalible in bug 65237
heikki: people are absolutely expecting DOM binary compatibility, that's why this bug was filed, and why I think we should fix it. We are not talking about the layout private interfaces here. I was initially contacted by rational.com, a developer there is building on the DOM API/ABIs shipped with Netscape 6. mozilla.org is not bound by every interface shipped before mozilla1.0 by a commercial derivative, of course. But the DOM APIs are as close to public as any, esp. their JS bindings. In this case, someone out there cares about the C++ bindings too. If we can avoid breaking such customers without standing on our heads and contorting the codebase too much, we should. /be
jst, can you cough up a more compleat back-out-or-move-forward patch, or should others help? Be nice to have an all-in-one diff -uN. /be
brendan: ok, good to hear that there actually was someone who really is depending on this. I was just concerned that we would be bending over backwards on the assumption that something might break. jst: once you fix this, would you mind reopening the bugs whose fixes got backed out so we know where we stand...?
It will be hard to verify the backout in transformiix since we currently have a regression that makes it impossible to load any documents (bug 73936). But make sure the code compiles and runs (both standalone and module).
Heikki, I'm handing this over to you since I don't have time to check this in before leaving for a two day east coast meeting, I won't be back before wednesday evening, please get reviews and approval (Brendan?) and check this in.
Assignee: jst → heikki
Would be better to #if 0 the whole GetBaseURI function in nsGenericDOMDataNode, like elsewhere. I can do that. r=heikki for everything except the Transformiix change. Transformiix change is simply a backout of the previous change to that one function, but can NSI_FROM_TX(Node) be removed safely, what is the function of that? Jonas, Peter, anyone?
Status: NEW → ASSIGNED
jag, can you review the nsContextMenu.js change? heikki, if you #if 0 more, could attach a new complete patch and I'll sr=? Thanks, /be
The context menu change is just a backup of my earlier change (so I do feel confident/competent in reviewing it;). The #if 0 change I suggested was just moving the #if 0 code *inside* the GetBaseURI to engulf the whole function, same as everywhere else. I just pulled and I am recompiling so it will take a little while to apply the patch, compile to check and get new diffs. (Hmm... has the whole world changed since Friday, it is rebuilding just about everything!)
The transformiix part of this ok. It's not pretty, but it works. So it's ok until we can get baseURI from mozilla again
sr=brendan@mozilla.org, then. asa or chofmann, please a=. /be
ok. checkin.
Checked in, marking fixed. jst watching the checkin (flight problems, he'll be here until the evening). I have reopened bug 72522 and bug 48217 and reassigned them to jst. Those can be fixed again once the DOM conversion branch lands. There is not a bug that covers the change in the Transformiix code (it was a partial back out of a bigger checkin).
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
verifying... *sob* I miss .baseURI already :(
Status: RESOLVED → VERIFIED
Component: DOM: Core → DOM: Core & HTML
QA Contact: desale → general
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: