On IE, we know if the onLoad event has already been triggered, however on Firefox/Mozilla, we don't know. Therefore, we can set up our code in an iframe to handle the onLoad event for the parent frame after onLoad has been triggered, rendering the event handler useless. However, publisher specifications often require the onLoad event to be triggered prior to doing certain things. Cases where this arise is where content is loaded in an iframe that must wait until the onLoad event on the parent document is triggered. This issue would be solved by creating the equivalent of IE's document.readystate == "complete" flag in Mozilla core.

fyi, Opera and Safari also support readyState.

See also bug 300992.

For a practical business case of why this is useful: I work for a rich media vendor, and as part of our industry's standard "polite download" specification, we don't load our content until after onLoad is called. However, there are cases with some publisher networks, that we can't guarantee our ad will be written out prior to the document being loaded, especially when we are served in iframes. For this reason, we are forced to sometimes have our content not display on Firefox because otherwise we'd be breaking our own specification. Then there are haphazard methods, like setting a timer and assuming after 5 seconds, if onLoad hasn't been called, that the page has already loaded. Obviously this isn't a concrete method of doing things, and our clients don't like wishy-washy results. Can someone with check-in privileges give indication of whether or not a readyState addition would be accepted if written properly?

Boris, David: Do you believe document.readyState would be accepted if a patch is written well to add this functionality?

test page for testing functionality

You're asking the wrong people, I think.... ccing the right ones.

I'd think that if someone wrote an implementation we'd accept it. However I have to say I don't understand your description of the usecase at all. Would be nice with a short description of what you are trying to do, and then a suggested solution that requires .readystate implemented.

I maintain code that runs on the top 100 publishing sites and many others. Publishing sites require 3rd party content (e.g. video players, advertisements, etc) to wait until the onLoad function is called prior to doing anything bandwidth-intensive. However, in some cases onLoad has already been called before the JS for the 3rd-party content is written out. There's no way to know that onLoad has already been called with Firefox (DOMContentLoaded isn't the same thing), and this 3rd party content will therefore wait for something to be called that never will be (because it's already been called). The only solution right now is to break the publisher specifications, or to do a timeout, hoping that the page will be loaded in some arbitrary amount of time -- e.g. 3 seconds. Neither breaking the specifications nor doing some hack is a valid solution (hacks aren't a very good choice for code that runs on the top100 publishers plus hundreds of others). In the case of online advertising (one form of 3rd party content), agencies combined probably lose millions of dollars a year worth of exposure time because of this deficiency that Firefox has but IE doesn't. This isn't acceptable in our industry, and I'm forced to constantly tweak the hack in specific ways for some publishers to get as much exposure time for our clients as possible.

Here's the code for the use case. The conditional chcks to see if document.readyState is "complete", which signifies that onLoad has already been fired and it's safe to start loading the rest of the 3rd-party content immediately. Otherwise, an event handler is created so the loader script can wait until onLoad is fired. // We are required to wait until onLoad has already been called prior to writing // out our 3rd party content. Since onLoad event may have been fired prior to // writing out the 3rd-party JS file that contains this script check whether onLoad // has already been called. if(document.readyState && document.readyState == 'complete') { onLoadHandler(); // onLoad has already fired, call onLoadHandler directly } else { // onLoad hasn't been called yet, create an event handler if (window.addEventListener) window.addEventListener("load",onLoadHandler,false); else if (window.attachEvent) window.attachEvent("onload",onLoadHandler); } // This onLoadHandler should only be called after onLoad event is fired function onLoadHandler() { // Write out bulk of 3rd-party content }

Feel free to provide a patch!

I've written a "patch" for this bug in the form of a really simple Firefox extension. (All it does is attach a load event listener to the "appcontent" browser object, and then set document.readyState = complete in its implementation.) With that said, I'm going to guess that this isn't the "right" way to fix this bug; an appropriate fix probably lives in Gecko somewhere (but I have no idea where). If somebody (Boris?) could give me a tip as to where to get started on this, I'd be happy to submit a patch. As for why I needed it, I'm a developer on Selenium. http://selenium.openqa.org. Selenium is a JavaScript UI testing framework that works in all browsers. We need to be able to automatically detect when a pop-up window is finished opening (but we don't/can't control the content of the window itself). With document.readyState, it's easy to handle this case: just wait for document.readyState to be complete. But without it, it's impossible to be sure whether a pop-up window is done loading. You can attach a load listener, but if you attach the load listener after the window is done loading, your listener will never get called (so you can't be sure it's done loading yet). For frames, it's easier, because you can attach a long-lived load listener to the frame object itself, but if you attach a load listener to a pop-up window, your load listener gets erased after unload, so it won't get called if/when the window re-loads.

This XUL overlay certainly isn't the "right" way to fix this bug; I'd be happy to re-code it if only I knew where to begin.

You could just add a readyState attribute to nsIDOMNSDocument, I guess. And implement it in nsDocument.cpp. That said, there are existing bugs on the mess with load listeners in popup windows; fixing those might be a better (though harder?) approach for your problem...

I've attached a patch (applied to the 1_8 branch, the Firefox 2.0 branch) to nsDocument that adds a "readyState" attribute to the document. In a separate attachment, I'll attach a test.html file that shows how it works. Note that the test.html file doesn't work exactly the same way in IE and in my patch: in IE, readyState is set to "complete" before the "load" event is fired; here, I set readyState to "complete" only *after* the load event is fired (it's set in OnPageShow). This was because nsDocument doesn't actually fire the real "load" event; that's handled by nsDocumentViewer's LoadComplete command. nsDocumentViewer fires the "load" event and then later calls "OnPageShow" on the document object; I didn't want to expose a special hook for nsDocumentViewer to directly modify the document's readyState (the attribute is marked "readonly" in the IDL) so I just waited until OnPageShow to set the readyState. While I imagine someone could use my work to fix bug 300992, I didn't do so here, mostly because I'm new to this code, and didn't want to do anything possibly tricky (like fire an event). Someone more confident than me could easily add support for onreadystatechange based on what I've done here. If this patch passes review, I'll happily generate another patch that could be applied to the SeaMonkey trunk.

You can use this test.html file to see the effect of patch252855 on this bug. In FF 2.0.0.1, all alerts simply print "undefined". With the patch applied, the document begins in a "loading" state, then enters an "interactive" state after DOMContentLoaded. During "load" the state is still "interactive"; immediately after "load", the state is set to "complete". In IE, the document is already in an "interactive" state by the time the script tag runs, and is already set to "complete" by the time "load" occurs.

I'm not going to be able to review this in a reasonable timeframe. Please try sicking or jst or peterv?

Comment on attachment 252855 [details] [diff] [review] Patch to add readyState property to documents My main concern with this patch is that I think we want something that mimics what IE does better. The reason to implement this is after all IE compat. The MSDN documentation is pretty lacking in describing when the property transitions into the various states, so it'd be great if someone could attach some testcases that work in IE. This includes documents that are created using the DOM and loaded using document.load (which is the only way you can reach some of the states I think). Please create a trunk patch instead since this bug does not meet branch criteria (not a crasher, not a security issue, etc).

Thanks for your prompt review. A couple of comments/questions: 1) As noted earlier in the bug thread, IE compatibility is *not* the main purpose of this patch (at least, for me). The reason I need document.readyState is to detect when a pop-up window is finished loading. IE compatibility is frosting, in my opinion. With that said, I agree that it's desirable to make the patch more IE compatible in that regard, so I'll continue investigating the matter. 2) As far as I can tell, the nsIDocument has no idea when a normal HTML document is finished loading (the "load" event is fired by the nsDocumentViewer). One way to handle this would be to make the readyState property writeable, so nsDocumentViewer could go update it directly, but that's very hackish, IMO. Can you suggest another way to handle this? Can/should the nsDocument try to listen for and handle the page load event? Is there another (better) way?

Well, since this is an IE property the rule of thumb should be that we follow IE, that'll help a lot more developers out there. You could simply add a method on nsIDocument that DocumentViewerImpl::LoadComplete can call before firing the event. But, again, we need to start by getting some testcases going. Everyone interested in getting this bug fixed should be able to help out with that. These testcases are needed since we want to add them to the testsuite anyway.

(In reply to comment #21) > Well, since this is an IE property the rule of thumb should be that we follow > IE, that'll help a lot more developers out there. > > You could simply add a method on nsIDocument that > DocumentViewerImpl::LoadComplete can call before firing the event. > > But, again, we need to start by getting some testcases going. Everyone > interested in getting this bug fixed should be able to help out with that. > These testcases are needed since we want to add them to the testsuite anyway. > Maybe someone should ask Hixie to come and offer some advice (maybe he knows a little more about what's going on in IE and what should be done?)

(In reply to comment #21) > But, again, we need to start by getting some testcases going. Everyone > interested in getting this bug fixed should be able to help out with that. > These testcases are needed since we want to add them to the testsuite anyway. Could you be more specific about what kind of test cases you're looking for? There are already two test HTML pages attached to this bug.

I'm going to go out on a limb and claim that the MSDN documentation is incorrect (or, at least, it's not true of IE6). Specifically, I can't find any way to get a document in IE to have the "uninitialized" or "loaded" readystate. Specifically, in IE6.0.3790.1830 (SP1) on Windows Server 2003 SP1, I tried attaching to the "onreadystatechange" event, and found that in my various experiments, "onreadystatechange" would only be called three times: "loading", "interactive" and "complete". I tried experimenting with longer documents, documents with images, documents which included long external JavaScript files, etc. to no avail. Jonas surmised(?) that the only way to get into some of the states on IE was to use document.load(). I tried that too... using "new ActiveXObject('Microsoft.XMLDOM')" I found that the object initially had a numeric readyState of 4, not a string-based readystate. Binding to "onreadystatechange" on that object did the "right thing", as I got called 4 times, with the readystate as 1, 2, 3, and 4. (Note that it was never 0.) The readyState numeric property on XMLHttp objects is notoriously quirky. http://www.quirksmode.org/blog/archives/2005/09/xmlhttp_notes_r_2.html In my opinion, the reason this has been allowed to continue is because nobody needs to wait for the "loaded" (2) readystate. People want to wait for either "interactive" or "complete". (Note that even the summary of this bug only requests support for readyState == "complete"... that's the only one I need as well.)

This maybe useful to someone working on this also. http://www.webreference.com/programming/javascript/domwrapper/4.html its a DOM Document Wrapper for implementing the readyState attribute and onreadystatechange event.

(In reply to comment #25) > This maybe useful to someone working on this also. > http://www.webreference.com/programming/javascript/domwrapper/4.html its a DOM > Document Wrapper for implementing the readyState attribute and > onreadystatechange event. Ironically, this only supports readystate 1 (loading) and 4 (complete), just like my patch. :-)

(In reply to comment #26) > Ironically, this only supports readystate 1 (loading) and 4 (complete), just > like my patch. :-) > That's true. I didn't notice that :) I've got some queries on exactly what MS means with their spec here. For example I can see how initialized would apply to say a textarea or whatnot but how about input type="radio"? Would we check if its default selected or something? Another problem we have is |interactive|. What would we do if the element is hidden or disabled? The user couldn't interact with it so what would we do here?

I'd never noticed that it applied to anything but documents. (It certainly makes sense in *that* context.) Regardless, all I'm hoping to do in this bug is get document.readyState to work. :-)

(In reply to comment #28) > I'd never noticed that it applied to anything but documents. (It certainly > makes sense in *that* context.) Regardless, all I'm hoping to do in this bug > is get document.readyState to work. :-) > According to the documentation it applies to the objects listed at the bottom of the MSDN page. I guess the first job would be to work out what IE does in various situations (iframes in a page, objects, applets etc.) and then create a model so that Firefox can do the same right?

If so, I'd suggest filing a separate bug for that behavior. This bug is about supporting document.readyState, which I'd say is considerably more important than supporting .readyState for the other objects on the page. (Not to mention the fact that the behavior you're suggesting implementing is almost entirely undocumented, appearing on a page of MSDN documentation that, according to my experiments, is actually wrong!)

(In reply to comment #30) > If so, I'd suggest filing a separate bug for that behavior. This bug is about > supporting document.readyState, which I'd say is considerably more important > than supporting .readyState for the other objects on the page. > > (Not to mention the fact that the behavior you're suggesting implementing is > almost entirely undocumented, appearing on a page of MSDN documentation that, > according to my experiments, is actually wrong!) > I agree on that. I've talked with Hixie on IRC and am hoping he'll be able to enlighten us a little on what should be done here. If we do implement the object.redyState attribute then it will require extensive testing with IE if we are to maintain a compatibility but with document.readyState it shouldn't be do bad (hopefully).

I'm fine with adding this to HTML5, btw, someone just needs to do the reverse-engineering work (or, mail the whatwg list and I'll do it, but I won't do it any time soon).

Assuming we're just talking about document.readyState, could you be more specific about what additional reverse engineering you have in mind? document.readyState == complete is the only property value that is consistently supported (it's available in IE, Opera and Safari/WebKit/Konqueror and Opera); the other property values are quirky. If I were writing this up as a part of HTML5, I'd describe it as a property of "document" that MUST be set to "complete" when the document is finished loading. It MAY have other values, including "loading" while the document is loading and "interactive" when the page is available for user interaction but not finished loading.

A (really) quick test here found this: (In reply to comment #33) > If I were writing this up as a part of HTML5, I'd describe it as a property of > "document" that MUST be set to "complete" when the document is finished > loading. It MAY have other values, including "loading" while the document is > loading and "interactive" when the page is available for user interaction but > not finished loading. > We should also use uninitialized here (e.g. the visual page hasn't started loading yet say if called from the pages header). If we can get this one working then it shouldn't be as bad to deal with the object.readyState attribute (assuming we can iron out the little problems with style and wahtever else may crop up). We could always mark that bug as dependant on this one and then it can be worked on after we have a working document.readystate. What sort of tests are you looking for Hixie?

Dan: things like whether it changes before, during, or after the load event, as the default action or always, whether it applies only to HTML or to anything, how it works when the document is actually an image, a plugin, etc, whether it's readonly, replacable, what exceptions fire when you set it, how it interacts with document.open()/document.write(), etc, etc, etc.

document.readyState is read only (throws "Object doesn't support this action of you try to modify it) which seems to be the correct action.

I'd be fine with only supporting the "complete" state and whatever it is before reaching the complete state if that is what is implemented in many other browsers. However we still need testcases to see when exactly IE and other browsers switch states and which states it switches between. See comment 35

A test of document.load functionality. This test is not cross-browser compatible; on other browsers, document.load is implemented very differently. (Specifically, in IE, when you load a document with document.load, it has a numeric readyState, instead of a string-based readyState like HTML documents.) In my patched trunk version, I see the following pop-ups when running this test: ---- first created: uninitialized document loaded: complete ----

Updated version of a test I submitted previously. This test includes no images or other content to load after the HTML document loads. It does four things: 1) pops-up an alert at the top of the <script> tag, reporting on document.readyState 2) Binds to the "DOMContentLoaded" event (if available) and reports document.readyState 3) Binds to the "ReadyStateChange" event (if available) and reports document.readyState 4) Binds to the load event and reports document.readyState. When I start IE like this: "iexplore c:\simpletest.html" I see this: ---- script tag: interactive readystatechange: complete load: complete ---- But then, when I click "Refresh", I see this: ---- script tag: loading readyStateChange: interactive readyStateChange: complete load: complete ---- Only IE showed a difference between launching the app directly and refreshing. (Weirdly, IE would sometimes show both the alerts for "interactive" and "complete" simultaneously.) In Opera 9.0.1 (same Windows machine), I see this: ---- script tag: interactive DOMContentLoaded: complete load: complete readyStateChange: complete ---- In my patched version of Minefield trunk, I see this: ---- script tag: loading DOMContentLoaded: interactive load: complete ---- In Safari 2.0.4 419.3 for Mac OS X 10.4.7 (Intel), I see this: ---- script tag: loading load: complete ----

A test of document.readyState in XSL-transformed documents. The test includes a testTransformable.xml file with a <?xml-stylesheet?> directive pointing to test.xsl. test.xsl just literally spits out simpletest.html, attached in comment #39. As noted in that comment, document.readyState behavior can be quirky across browsers (except for the "complete" state). Behavior during XSLT transformation is even quirkier. In my opinion, since the "complete" state is consistent across browsers, this behavior may be considered acceptable. It may also be considered as a regression test to ensure I didn't break anything by modifying nsXMLDocument. In IE, when launching the HTML file directly from the command line, I see this: ---- script tag: interactive readyStateChange: complete load: complete ---- but when I hit Refresh, I see: ---- script tag: loading readyStateChange: interactive readyStateChange: complete load: complete ---- On IE, this matches the behavior in simpletest.html, including the funny business with the two alerts (interactive & complete) popping up simultaneously. In Opera 9.0.2, I see this: ---- script tag: complete load: complete load: complete readyStateChange: complete ---- In Opera, unlike in simpletest.html, I don't see a DOMContentLoaded event (even though Opera supports it), and instead we see the load event fired twice. (This is almost certainly an Opera bug.) In my patched version of Minefield trunk, I see this: ---- script tag: uninitialized load: complete ---- In my patched Minefield, unlike in simpletest.html, I don't see a DOMContentLoaded event, and the document state is "uninitialized" at first, but it is "complete" before the load event. Finally, in Safari 2.0.4 419.3 for Mac OS X 10.4.7 (Intel), I see this: ---- script tag: loading load: complete ---- This matches the behavior of simpletest.html.

Looks like its going to be quite complicated to get a consistent behaviour across browsers then. Do you have any information about how document.write, dynamically generated content (e.g. DOM) and things like frames affect this by any chance? :) Looks like we may need Hixie back a little later to help define how this will work seeing as we'll need some compatibility with IE, Opera etc.

Comment on attachment 253239 [details] [diff] [review] Trunk patch to add document.readyState property to documents Oops, meant to add comments to the patch attachment: This patch takes into account Jonas' feedback from comment #19. It is now a patch of the "Minefield" trunk rather than the 1_8 branch. Also, the patch now ensures that document.readyState is set to "complete" *prior* to firing the load event, which matches the behavior on IE, Safari, and Opera. In so doing, I introduced a new non-scriptable method on nsIDocument, "FinishLoad", which exists only to inform the document that it is completely finished loading (so it can update its readyState). As noted in earlier comments, the nsDocument otherwise has no idea when the full document (including images, etc.) is done loading. ::FinishLoad is called by nsDocumentViewer for HTML documents, and is called by nsXMLDocument for XML documents. I'm not sure if I'm totally happy with the name "FinishLoad" (it seems too close to "EndLoad"); I'm open to alternative suggestions as to what I should call it. For XML documents, nsXMLDocument fired the "load" event directly, but it called nsDocument::EndLoad only *after* it had fired the "load" event. In my patch, we now call nsDocument::EndLoad and then nsDocument::FinishLoad before firing the load event, which results in the correct behavior in the xmlReadyState.zip test attached in comment #38. I was a little nervous about this, but it doesn't seem to have broken anything, as shown in the tests attached to comment #38 and comment #40. The main test for this functionality is the attachment to comment #39. (Note that I typed Opera 9.0.1 accidentally, but I meant to type Opera 9.0.2.) Note that everything except the "complete" state is quirky across browsers. The results we see in my patched Minefield seem as reasonable to me as anything else.

Changing the order of the internal calls are bound to break something, so don't do it unless you have a really good reason to. I'd still like to see more tests before I review this. At the very least the things mentioned in comment 35, but the more tests the better.

This test needs to be run in the same directory as "simpletest.html" (in attachment 253236 [details]). If you thought the earlier test results were quirky, wait until you see these. Every browser except Safari had an effect where you'd see multiple JS alert pop-ups both at once (apparently one in each thread). More pop-ups would appear (again, overlapping each other) depending on which pop-ups you clicked on. Here, I use a notation of "*" to indicate two pop-ups that appeared simultaneously. Where a whole hierarchy of pop-ups appeared simultaneously, I would number them. So, in Minefield, "1" and "2" appeared simultaneously, and the item marked "*" under "1" appeared after clicking on "1"; the items marked "*" under "2" appeared one-after-another after clicking on "2". The most complex was Opera, where two pop-ups 1 and 2 appeared, but 1 had no further pop-ups until 2 was dismissed. After 2 was dismissed, 2a and 2b appeared. After clicking on 2a, you'd see the * items under 2a (in order); after clicking on 2b, you'd see the * items under 2b (in order). It's actually not all that weird once you wrap your head around it, but it made the test results difficult to record. Anyway, here were the test results on my machine: IE6 parentFrame script tag: interactive script tag: loading *readyStateChange: interactive *readyStateChange: complete parentFrame readyStateChange: complete parentFrame load: complete load: complete Opera 9.0.2 parentFrame script tag: interactive 1)parentFrame DOMContentLoaded 2)script tag: interactive 2a)parentFrame load: complete *parentFrame readyStateChange: complete 2b)DOMContentLoaded: complete *load: complete *readyStateChange: complete Minefield with my patch 253239 parentFrame script tag: loading 1)parentFrame DOMContentLoaded: interactive *parentFrame load: complete 2)script tag: loading *DOMContentLoaded: interactive *load: complete Safari 2.0.4 parentFrame script tag: loading script tag: loading load: complete parentFrame load: complete

This test is identical to simpletest.html, except after the load event, we add a non-cached image using document.body.appendChild. The results are identical to the results shown in comment #39 until after the load event. After that, all browsers except Opera add a new message: "after addImage: complete". Opera behaves differently, like this: script tag: interactive DOMContentLoaded: complete load: complete after addImage: interactive readyStateChange: complete

This test is identical to imagetest.html in comment #46, which is very similar to simpletest.html in comment #39. After the load event, we add a non-cached image using document.writeln. We also add a "readyState?" button which I manually click on after the image has finished loading and the spinner/flag/etc. has stopped moving. The results are identical to the results shown in comment #39 until after the load event. After that, behavior is quirky: IE6: --- [...] load: complete after addImage: loading --- Then I click on the "readyState?" button and see: "interactive" Note that, on IE6, the document *never* returns to "complete" state, even long after the image has finished loading. Opera 9.0.2: --- [...] load: complete after addImage: interactive readyStateChange: complete --- Then I click on the "readyState?" button and see: "complete" Thus, on Opera, behavior is identical between adding an image using the DOM (comment #46) and adding an image with document. write. Minefield with my patch --- [...] load: complete after addImage: loading --- Then I click on the "readyState?" button and see: "complete" Safari 2.0.4 --- [...] load: complete after addImage: loading --- Then I click on the "readyState?" button and see: "loading" Note that Safari (like IE) *never* returns to the "complete" state, even after the image has finished loading.

This is probably overkill, but just to be sure, this test is exactly like the test attached to comment #47, except I call "var mydoc = document.open('text/html')" immediately before calling mydoc.write. It has the exactly the same results as the test attached to comment #47 on all of my tested browsers.

(In reply to comment #35) Whew! I've added a lot of tests, and I think I've satisfied all of the tests anybody on this bug thread have suggested. Please let me know if I've overlooked any. :-) Answering Hixie's questions: > whether it changes before, during, or after the load event On IE, Opera and Safari document.readyState changes to "complete" before the load event, as shown by the simpletest.html test attachment to comment #39. (Note that on Opera, the readyState changes and the "load" event fires before the "onReadyStateChange" event fires.) As I noted in comment #33, I believe this is the only fact that could be consistently standardized, based on my browser experiments shown here. > as the default action or always Not sure if I understand this one. Let me know if the tests submitted don't already answer this question...? > whether it applies only to HTML or to anything, how it works when the document is actually an image, a plugin, etc The only thing that has a consistently implemented readyState is the "document" DOM object, which (as far as I know) can be either an HTML or XML-based document, but not an image, a plugin, etc. As discussed in comment #26, MSDN claims (without any further documentation) that .readyState can apply to an absurdly long list of things, including INPUT tags, (which, IMO, makes no sense) but that's not the least bit standardized (and shouldn't be). MSDN claims that it applies to: document, FRAME, IFRAME, IMG, INPUT type=button, INPUT type=checkbox, INPUT type=file, INPUT type=hidden, INPUT type=image, INPUT type=password, INPUT type=radio, INPUT type=reset, INPUT type=submit, INPUT type=text, LINK, SCRIPT, TABLE, TBODY, TD, TFOOT, TH, THEAD, TR. > whether it's readonly, replacable, what exceptions fire when you set it As Ryan Jones shows in comment #36, document.readyState is read-only, throwing the TypeError "Object doesn't support this action" when you attempt to modify it. > how it interacts with document.open()/document.write() See tests attached to comment #47 and comment #48. > etc. etc. etc. I hope I know what "etc. etc. etc." is, and that it's simply what happens when you modify the document using the DOM, as in the test attached to comment #46. :-) (If there are other questions/tests of a broader nature that you haven't mentioned here, let me know.)

(In reply to comment #44) > Changing the order of the internal calls are bound to break something, so don't > do it unless you have a really good reason to. Agreed. I hope my XML/XSL tests provide at least prima facie evidence that there's no harm done. There's three things that need to get done in nsXMLDocument::EndLoad... in my patch they're in this order: 1) Call nsDocument::EndLoad, which fires the DOMContentLoaded event, and sets readyState="interactive" 2) Call nsDocument::FinishLoad, which sets readyState="complete" 3) Fire the "load" event Looking at the currently checked-in trunk, there is no step 2, and nsXMLDocument::EndLoad does this: 3) Fire the "load" event 1) Call nsDocument::EndLoad, which fires the DOMContentLoaded event [Frankly, I think that's backwards, even not taking document.readyState concerns into account. Shouldn't DOMContentLoaded come before Load?] If we wanted to be extra safe at the expense of being a bit hackish, we could add code to make sure that if readyState=4(complete), it cannot decrease [can't go back to 3(interactive)]. This is in agreement with the behavior of IE and Safari (but not Opera), as in the test attached to comment #46. Then I could call it like this: 2) nsDocument::FinishLoad 3) Fire the "load" event 1) Call nsDocument::EndLoad, which fires the DOMContentLoaded event > I'd still like to see more tests before I review this. At the very least the > things mentioned in comment 35, but the more tests the better. Well, I've checked in four more cross-browser tests since your comment #45. Definitely let me know if there's any more tests you'd like to see...

> (Specifically, in IE, when you load a document with document.load, it has a > numeric readyState, instead of a string-based readyState like HTML documents.) Probably has to do with them being handled by completely different DOM implementations -- HTML is handled by Trident, while document.load is handled by MSXML. There are all sorts of things that the two do differently...

Just checking in on this bug... Is anyone still waiting for additional material from me? As far as I know, with the four additional tests I submitted, my patch is ready for review.

> If we wanted to be extra safe at the expense of being a bit hackish, we could > add code to make sure that if readyState=4(complete), it cannot decrease [can't > go back to 3(interactive)]. This is in agreement with the behavior of IE and > Safari (but not Opera), as in the test attached to comment #46. When do we with the current patch to from 4 to 3? Doing what IE does always feels safer...

With the current patch, there is no code in place to *guarantee* that the document.readyState never goes from 4 to 3, but my tests above show that in practice we never actually DO go from 4 to 3. In principle, it's possible under the current patch that a document.readyState *could* go from 4 to 3 if someone were to call nsDocument::FinishLoad and THEN call nsDocument::EndLoad. I know of no code checked in today that would do that, but somebody *could* write it, and that person would introduce a bug if they did that. (I do not take this to be a great risk. It's much more likely [hypothetically speaking] that someone would simply forget to call the new ::FinishLoad method than it is that they would know to call it, but foolishly call it before ::EndLoad.) In comment #50, I had imagined that if we DID implement something like readyState safety checking, we could hackishly rely on the safety checking in nsXMLDocument. I think that's hackish because, IMO, nsXMLDocument is doing things in the wrong order in the checked-in trunk today (it fires "load" and then later fires "DOMContentLoaded", which makes no sense). With readyState safety checking, we could leave the incorrect ordering in place in nsXMLDocument, but feel confident that the incorrect ordering doesn't cause the readyState to go from 4 to 3.

Comment on attachment 253239 [details] [diff] [review] Trunk patch to add document.readyState property to documents >Index: content/base/public/nsIDocument.h >+ /** >+ * Inform the document that it is completely finished loading (so it can update its readyState) >+ */ >+ virtual void FinishLoad() = 0; Change the name to LoadFinished since it's a notification, not a request to do something. >Index: content/base/src/nsDocument.h >+ PRUint32 mReadyState; >+ enum { READYSTATE_UNINITIALIZED, READYSTATE_LOADING, READYSTATE_LOADED, READYSTATE_INTERACTIVE, READYSTATE_COMPLETE }; Make mReadyState be of the enum type. Also explicitly make READYSTATE_UNINITIALIZED be 0 since you're relying on that. > > // A map from unvisited URI hashes to content elements > nsTHashtable<nsUint32ToContentHashEntry> mLinkMap; > // URIs whose visitedness has changed while we were hidden > nsCOMArray<nsIURI> mVisitednessChangedURIs; > > // Member to store out last-selected stylesheet set. > nsString mLastStyleSheetSet; >Index: content/xml/document/src/nsXMLDocument.cpp >=================================================================== >RCS file: /cvsroot/mozilla/content/xml/document/src/nsXMLDocument.cpp,v >retrieving revision 1.252 >diff -u -8 -p -r1.252 nsXMLDocument.cpp >--- content/xml/document/src/nsXMLDocument.cpp 22 Nov 2006 18:27:53 -0000 1.252 >+++ content/xml/document/src/nsXMLDocument.cpp 29 Jan 2007 22:37:57 -0000 >@@ -624,17 +624,19 @@ nsXMLDocument::StartDocumentLoad(const c > } > > void > nsXMLDocument::EndLoad() > { > mChannelIsPending = PR_FALSE; > mLoopingForSyncLoad = PR_FALSE; > >+ nsDocument::EndLoad(); > if (mLoadedAsData || mLoadedAsInteractiveData) { >+ nsDocument::FinishLoad(); Why is this call needed in nsXMLDocument but not in nsHTMLDocument? You can't rely on that the contentviewer always being there, for example DOMParser creates HTML documents and so can the XSLTProcessor JS object.

(In reply to comment #55) > Why is this call needed in nsXMLDocument but not in nsHTMLDocument? You can't > rely on that the contentviewer always being there, for example DOMParser > creates HTML documents and so can the XSLTProcessor JS object. We discussed this briefly in IRC about an hour ago. (begin summary) In nsXMLDocument, I'm only calling ::LoadFinished (as we're now calling it) for data documents, which fire their own "load" event during ::EndLoad. My general philosophy for deciding where to add calls to ::LoadFinished was to simply add the call wherever the "load" event is fired. [We also agreed that I would add assertions that document.readyState should not decrease, and that I should rewrite the tests to avoid using alerts.] As for the more general case where I can't always rely on the contentviewer, we discussed a few alternatives. I pointed out that nsDocument doesn't *know* when it is done loading (i.e. when its readyState is complete) or when to fire the "load" event; nsDocument relies on the contentviewer to fire the "load" event, so it makes sense to depend on it to call ::LoadFinished as well. Jonas remarked that we probably don't have a way to guarantee that the "load" event gets fired when there's no contentviewer, so we probably *don't* fire "load" for all documents; this is arguably a bug in itself. With that said, Jonas said that he didn't want to leave any documents forever in the "interactive" state, no matter what IE does. I don't feel confident enough to refactor this code to guarantee a "load" event gets fired; he suggested that we might try to check to see if there is a contentviewer, writing something like "if (!thereIsAContentViewer()) { LoadFinished(); }" at the end of nsDocument::EndLoad. We both tried searching through the API/docs to find a way to test to see if there's a contentviewer attached to the current document, but we weren't able to find a way before the conversation ended. (end summary) With that said, upon further consideration, I feel that even if we COULD figure out how to detect whether a contentviewer was present, I'm not so sure that we'd WANT to force documents into the "complete" state in cases where we don't fire a "load" event. IMO, the semantics of setting document.readyState to "complete" is that the readyState should not be "complete" until we're firing the "load" event. (Indeed, my whole interest in this bug is that monitoring document.readyState is the way to detect [after the fact] whether onLoad has finished.) If the document is never going to load due to a bug, that's a real problem, but it should be resolved by making sure that the document gets loaded; we shouldn't just cheat and go straight to "complete" IMO. Jonas made a remark that I agree with 100%, that neither of us wants to make a **** implementation of this property. In this case, I think fidelity to the cross-browser semantics of the property means tying it strictly to the "load" event, even (perhaps especially) in cases where the "load" event will never fire. As a final remark, I think tying readyState=="complete" to the "load" event will make it easier to QA "load" event bugs. IMO, the fact that it may expose a latent bug here is actually a feature!

So what's the conclusion here? What should the spec say?

As mentioned, implementing document.readyState is important for Selenium-type testing frameworks. I hope this feature makes it in eventually.

I'm wondering regarding HTML5 whether IE's implementation is too ambiguous, inconsistent and browser-centric to be more than a starting point. Other browsers' attempt to emulate seem to support this. I think Firefox should do its best to emulate IE's behavior for document.readyState for the same reason as it emulates innerHTML (albeit better), however I also believe that for HTML5, there are many ambiguities in their implementation to get passed, along with how the states are more specific to Internet Explorer than cross-browser models of how content loads. For instance, IE's readyState doesn't offer a way to check whether the DOMContentLoaded event was already triggered, even though it's now in the spec (http://www.whatwg.org/specs/web-apps/current-work/multipage/section-tree-construction.html#the-end). Kudos Dan on the amount of thought that has gone into this. I'm sure many people are going to appreciate finally being able to tell whether onload has already been called. Your analysis also gives me some insight into some frustrating bugs I've seen with IE and loading, e.g. 'Note that, on IE6, the document *never* returns to "complete" state, even long after the image has finished loading.'

Sorry for having been away from this bug for some time. Unfortunately it's got a lower priority since it's not a blocker for FF3. Below comments are somewhat from memory, so let me know if they are wrong: I'm still not feeling confident of this patch. I think what we should do is to initialize the member to the "complete" state in the constructor, but then make StartLoad switch it to whatever state is used for loading. That way documents created using other means will always end up in the "complete" state.

It's a pity I ran out of time to work on this earlier this year. (I presume now that if my patch were ready to submit today, we wouldn't see it in a release of FF until FF4 at the earliest, and probably more like FF5 or 6 since FF doesn't take trunk versions of Gecko on every major FF release...?) Fundamentally, the way documents are currently designed in Gecko does not guarantee that they will ever have a load event. In that case, it's ambiguous what exactly should happen to document.readyState. As I argued in comment #56, I think that documents that never fire a load event should never enter a "complete" state. In May, Jonas suggested that we attempt to detect cases where we know that the load event won't ever fire and force readyState to be complete in that case, but I don't think that's correct. In comment #60 Jonas suggests possibly making the readyState be complete initially... but it is officially not correct semantics for a document's readyState to start in the "complete" state; readyState should begin in the 0 (uninitialized) state. [With all that said, I would again argue that what we decide to do in ambiguous cases fundamentally doesn't matter here. As I think I've conclusively shown, the only thing you can be sure of cross-browser is that document.readyState will be "interactive" just before DOMContentLoaded and it will be "complete" just before the load event fires, and fortunately that's the only thing anyone in the world cares about.]

(In reply to comment #57) > So what's the conclusion here? What should the spec say? Thanks to Brian for finding the link to the relevant draft section. Section 8.2.5 ("The End") should change its last two paragraphs to say: ----------- [...] The user agent must then change the value of the readyState attribute of the Document to "interactive" and fire a simple event called DOMContentLoaded at the Document. Once everything that delays the load event has completed, the user agent must change the value of the readyState attribute of the Document to "complete" and fire a load event at the body element. ----------- Everything I just said is (or should be) totally unobjectionable, no matter what Jonas and I conclude re: documents that never have a load event. I further speculate that it might also be a good idea to put "onreadystatechange" into the HTML5 spec. My patch doesn't implement "onreadystatechange" (that's bug 300992) because cross-browser support for "onreadystatechange" is very quirky. (see my test results in comment #39) If we wanted to add "onreadystatechange" to HTML5, then we should add "and fire a simple event called readystatechange at the Document" in both of those paragraphs, like this: ----------- [...] The user agent must then change the value of the readyState attribute of the Document to "interactive" and and fire a simple event called "onreadystatechange" at the Document, and then fire a simple event called DOMContentLoaded at the Document. Once everything that delays the load event has completed, the user agent must change the value of the readyState attribute of the Document to "complete", fire a simple event called "onreadystatechange" at the Document, and finally fire a load event at the body element. ----------- But even that isn't uniform across browsers; for example, Opera fires onreadystatechange *after* the load event.

Can you ever see it in the 0 state in IE? If yes, when? If no, how can you then know that 0 is the correct initial state, and does it really matter. Even if we initialize the document to "complete" and set it to something else in StartLoad I think the user will ever be able to notice that, since the document can only be reached after StartLoad as been called. It would however insure that documents created using document.domImplementation.createDocument() aren't left in a non-complete state.

(In reply to comment #63) > Can you ever see it in the 0 state in IE? If yes, when? If no, how can you > then know that 0 is the correct initial state Actually, it turns out, yes. Your suggestion made me think of another test, which I just performed: <html><head> <script> window.alert(document.createDocumentFragment().readyState); </script> </head> <body> </body> </html> IE7 returns "uninitialized", the 0 state. Opera 9.2 returns "undefined". Safari 3 returns the empty string "". I'd tell you what happens on my patched Minefield, but I no longer have a dev environment set up with this patch applied. (I wrote this code 9 months ago) > and does it really matter. Definitely not! :-) I still have my opinions (I think leaving it as non-"complete" more correctly characterizes what's going on in documents that will never "load") but it's not that important. > Even if we initialize the document to "complete" and set it to something else > in StartLoad I think the user will ever be able to notice that, since the > document can only be reached after StartLoad as been called. > > It would however insure that documents created using > document.domImplementation.createDocument() aren't left in a non-complete > state. True... I guess it depends on what you think "complete" means.

Note that in IE .createDocumentFragment() erroneously creates a document. In Firefox (and I'd imagine in most other browsers) it creates a documentFragment, as per spec. documentFragments should not have a .readyState property, nor will they with your patch.

Finally returning to this bug again a year later. Now that FF3 is out, I think I should be able to submit a patch to "trunk" (http://hg.mozilla.org/mozilla-central/) and get it into some version of FF in next year or so. ;-) This zip file includes updated versions of all of my earlier tests, except now instead of using window.alert they write content into the DOM of the page, so they're suitable for automated testing. I've also included an HTML file describing the test results on all of the browsers I tested (IE7, Safari 3.1.1, Opera 9.50). The results are very similar to the results posted in comments above, so I won't repeat them here. (This bug thread is already horribly long!) I will, however, upload the test results as a separate attachment for easier viewing.

These are the HTML test results, extracted for easier viewing. Executive summary: All of these results are quirky. The only consistent result is that document.readyState is "complete" during onLoad. (This is the "golden rule" of document.readyState.) Opera usually disagrees from all the other browsers, though it always follows the golden rule. In a curious twist, IE even disagrees with itself! (see comment #39)

This patch incorporates all of Jonas' feedback from comment #55. As I remarked in comment #56, nsXMLDocument calls its own LoadFinished only for data documents which don't have a contentviewer. Otherwise, since the document itself requires a contentviewer in order to have a load event, the document also requires a contentviewer to update its readyState to "complete." HTML documents with no contentviewer will never have a load event, and so they will (correctly) never go to readyState="complete". I also did NOT add assertions that document.readyState never decreases, because it is (surprisingly) legal for document.readyState to decrease when you use document.write. One of the attached tests, imagewritetest.html (in the attachment to comment #66) demonstrates this. Adding assertions caused assertion failures during that test, so I backed out the assertions.

Additionally, something *else* has changed between 2007 and 2008: document.readyState was added to HTML5. Therefore, implementing this feature is now an HTML5 compliance issue. http://lists.whatwg.org/pipermail/commit-watchers-whatwg.org/2008/000652.html The "current document readiness" is defined here: http://www.whatwg.org/specs/web-apps/current-work/multipage/dom.html#current > Each document has a current document readiness. When a Document object > is created, it must have its current document readiness set to the > string "loading". Various algorithms during page loading affect this > value. When the value is set, the user agent must fire a simple event > called readystatechanged at the Document object. And the ultimate load behavior is defined here: http://www.whatwg.org/specs/web-apps/current-work/multipage/tree-construction.html#the-end My patch is in compliance with the new HTML5 specification. (Note that firing the readystatechanged event is a separate bug #300992 which should be addressed after the current bug is fixed.) Well, it's *mostly* in compliance. ;-) There's one bit of weirdness between the HTML5 specification and the MSDN documentation that formed the basis for this feature. According to the HTML5 definition of "current document readiness", 'When a Document object is created, it must have its current document readiness set to the string "loading".' That's in disagreement with the MSDN documentation. http://msdn.microsoft.com/en-us/library/ms534359(VS.85).aspx (Note that the URL attribute of this bug is old/broken.) > An object's state is initially set to uninitialized, and then to > loading. When data loading is complete, the state of the link object > passes through the loaded and interactive states to reach the complete > state. My patch sets the document.readyState to 0 = uninitialized by default, which is in agreement with MSDN but arguably against the HTML5 specification. We can change this if we prefer. In my defense, I think HTML5 should change to agree with MSDN on this point, and furthermore Jonas correctly pointed out in comment #63 that the initial state of this property doesn't matter. If document.readyState = "uninitialized" but there's no way to see it in that state, then it's harmless either way. [If an event fires in a forest, but no listeners are attached... ;-)] As the test results in the attachment to comment #67 show, there's really very little agreement between the browsers about anything except that the readyState should ultimately be "complete" and that it should be "something else" prior to that.

Over on the WHATWG group list I've posted my suggestion to amend HTML5 to say that the initial state is "uninitialized"; Hixie has pushed back a bit. I'm afraid that my patch is going to get rejected again for a *very* unimportant reason and that this will result in yet another six month turnaround, so I'm going to submit a second patch that initializes .readyState to "loading" in the nsDocument constructor... the reviewer may choose between the two patches if one or the other is more pleasing to him. :-) Here's the link to the WHATWG thread: http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2008-June/015186.html

And, just in case the reviewer decides that initializing to "complete" is the right way to paint this bike shed, here's a third patch that initializes the value to "complete". I disagree with this one, because I think documents that never have a load event should never have the "complete" readyState. However, at the end of the day I care so little about the initial state of this property that I'm happy to yield: if this is the only thing holding up this patch, then let compromise rule the day! :-) Note that this makes the XSL test incorrectly claim to have loaded long before the load event, like this: script tag: complete load: complete It also makes the XML load test go like this: first created: complete document loaded: complete I think that's silly, but if this is what it takes to get a + around here.. ;-)

Comment on attachment 326211 [details] [diff] [review] mozilla-central patch to add document.readyState property to documents So I finally looked at this patch. In general it looks good, however I think I'd prefer to initialize the readystate to "complete" so that if you create a document through the DOM (for example using implementation.createDocument) then you get a document that looks ready to use. Also, what is the purpose of the READYSTATE_LOADING state, you never seem to use it? Finally, why the change to nsDocument::OnPageShow?

Comment on attachment 326236 [details] [diff] [review] yet another mozilla-central patch to add document.readyState property to documents (initializing to "complete") Oh, noticed that the last patch does address one of my comments. >diff --git a/content/base/src/nsDocument.cpp b/content/base/src/nsDocument.cpp > nsDocument::nsDocument(const char* aContentType) > : nsIDocument(), > mVisible(PR_TRUE) > { >+ mReadyState = READYSTATE_COMPLETE; // So documents created with document.implementation.createDocument() will be complete ...But put this in the colon list rather than inside the body. Or, you can define READYSTATE_COMPLETE as 0 which means that you don't have to initialize mReadyState at all (all of nsDocument is zeroed out on allocation). Also, what is the purpose of READYSTATE_UNINITIALIZED, doesn't look like we can ever get into that state. Just remove it altogether.

(In reply to comment #72) > Also, what is the purpose of the READYSTATE_LOADING state, you never seem to > use it? No, we use it. It's the state we're in during ::BeginLoad. > Finally, why the change to nsDocument::OnPageShow? Good catch! That's a remnant of my first attempt at this patch that didn't have perfect fidelity to IE. It's harmless, and I left it in for safety, but just now I tried removing it and it seems fine. I'll resubmit three competing patches again, initializing the readyState in the colon lists. I will not, however, remove "unused" entries from the enum. My favorite version of the patch *does* use all five values. Your favorite version of the patch (the one that initializes the value to "COMPLETE") doesn't use all five values, but I think your favorite is wrong. I've given up hope that I'll convince you to change your mind in this bug. Instead, if/when this patch is ultimately accepted, I'll immediately turn around and file *another* bug and we can argue about it there. ;-)

Resubmitted my patch incorporating Jonas' comment #72. This the best patch, the one we should actually apply to the source tree.

Resubmitting a second version of my patch, incorporating Jonas' comment #72 and Jonas' comment #73. This patch initializes readyState to "LOADING" instead of "UNINITIALIZED". This patch isn't as good as patch 331139, but it's arguably more faithful to the HTML 5 spec (and less faithful to the MSDN documentation).

Resubmitting a second version of my patch, incorporating Jonas' comment #72 and Jonas' comment #73. This patch initializes readyState to "COMPLETE" instead of "UNINITIALIZED". This patch isn't as good as patch 331139 or as good as patch 331141, but Jonas has said he'd prefer it, and so I submit it as a compromise.

After some additional discussion on IRC, we agreed to another compromise: the readyState would initially be "uninitialized", but .createDocument would explicitly set the readyState to "complete", in agreement with Jonas' suggestion on the WHATWG list. http://www.mail-archive.com/whatwg@lists.whatwg.org/msg10495.html Additionally, I added fixes to XSLT so it would explictly set the readyState to "loading" (in createResultDocument) and "interactive" (in endDocument) at the right times, because XSLT doesn't call BeginLoad/EndLoad on the result document (though it arguably should). Additionally, XSLTProcessor also needs to explicitly set the readyState to "complete" when you use .transformToDocument. While testing this patch I discovered that there was a bug in my previous patch: immediately after calling .load on an XML document the readyState was still "complete" instead of "loading"; I fixed that by having nsDocument set the readyState to "loading" in "StartDocumentLoad" instead of in "BeginLoad". All tests now pass, including one additional XSLTProcessor test that I'll attach separately.

Added a test that creates a document using XSLTProcessor.transformToDocument. With the latest pach (331351) my results agree with Opera: the resulting document is in a "complete" state once it is transformed.

It would be great to get this landed in 1.9.1 - would be useful for web developers and extension developers (like Firebug).

Comment on attachment 331351 [details] [diff] [review] mozilla-central patch to add document.readyState property to documents r=jst for this, but Jonas really needs to have the final say here.

On IRC, mrbkap asked what revision this patch was based on. (stupid hg doesn't include this in the patch by default, apparently...) $ hg parent changeset: 15465:0e6655f7d896 tag: tip user: Brendan Eich <brendan@mozilla.org> date: Fri Jun 20 14:25:38 2008 -0700 summary: Fix bustage caused by over-aggressive TRY_BRANCH_AFTER_COND in STRICT_EQUALITY_OP (used by JSOP_CASE*).

Comment on attachment 331351 [details] [diff] [review] mozilla-central patch to add document.readyState property to documents Looks great, though please write some mochitests for both the normal and the XSLT case.

OK, here's an updated patch including the previously r+sr patch as well as four new mochitests incorporating earlier tests. All four tests pass on my machine. As a nice bonus, 3 out of the 4 tests are runnable on IE7, and those three pass on IE7.

I believe the HTML5 spec got updated to deal with all sorts of documents, does this patch implement what the spec now says?

Can you provide a more specific pointer? I just did a ctrl-F for readyState and saw references to WebSocket and Media elements (which don't have anything to do with this change). [As an unprofessional aside, I've been trying to get this patch landed for two years. Every other major browser already supports document.readyState. I'd hate for it to get held up yet again for a new kind of document invented in HTML5. For the love of god, just check it in, man! :-)]

Well, most of that time was spent arguing about what the behavior for the property should be. The patch that ultimately got r/sr+ wasn't attached until july last year. I'll also note that there are still dead code in the patch (the 'loaded' state), something I specifically requested not to have. The only reason I still marked the patch sr+ was because I was tired of arguing over the patch and felt that it was more important to get the feature into the hands of web developers. So I did in fact bend the rules in your favor. I am however sorry it took me 2 months to mark it sr+. That said, I don't know why the patch wasn't checked in after the patch received r/sr and got the checkin-needed keyword. We used to have people vacuuming bugzilla for reviewed patches with that keyword but it appears that hasn't happened here. Were we in feature freeze already then maybe? Here is a pointer to what the HTML5 spec says: http://www.whatwg.org/specs/web-apps/current-work/#current-document-readiness Specifically it says that documents that aren't associated with a parser should immediately have the readystate 'complete'.

Ah, yes, I see now. This change happened about a week ago in r2599 http://html5.org/tools/web-apps-tracker?from=2598&to=2599 My patch agrees with HTML5, but the mochitests in the patch don't explicitly verify the new language from r2599. I've attached a revised version of test_247174_xslp.html that verifies that immediately after calling .createDocument, the readyState is "complete". (By adding one new assertion.) I'm sorry about overlooking the dead code. In fact, I'm sorry for tossing in that unprofessional aside... whining doesn't help anything. My goal is not to make somebody apologize; my goal is to get this bug fixed.

Would be great if you could attach a mq patch that contains all latest coderevisions and tests. If you do that I'll lookit it all over and check it in if it looks good.

OK, here's an updated unified patch. It's not an "mq patch"; I don't know how to make one of those. When I asked on #developers roc said "I don't think he really means that. I think he just wants a single patch with all new files, code changes etc" So if you don't like the patch format, blame roc. ;-) $ hg par changeset: 23417:4f196841ba1a tag: tip user: Vladimir Vukicevic <vladimir@pobox.com> date: Wed Jan 07 11:50:14 2009 -0800 summary: b=469916, lcms error when trying to open 0-sized profile; r=bholley

The benefit of an mq patch is that it contains the committer's name (if done right), as well as the commit message. You can get an equivalent effect by just committing your changes to your local repository and then creating a patch with hg export.

oops, accidentally converted some UNIX->DOS line endings, this one's better.

OK, here's an "hg export" patch. I actually went through the trouble of cooking up an mq patch, but it is in fact byte-for-byte identical with the "hg export" patch; serves me right ;-)

Comment on attachment 356251 [details] [diff] [review] mozilla-central patch to add document.readyState property to documents Checked in

Checked in! Thanks for the patch!

This patch didn't change the interface id, did it?

I don't know what an interface ID is... do you mean the UUID in nsIDOMNSDocument? I didn't change that. The patch is quite short; you can skim through it easily to see if I changed an interface ID.

> do you mean the UUID in nsIDOMNSDocument? I didn't change that. Right. You should have, is the point, since the change changes the vtable (and in fact inserts stuff in the middle of the vtable). Or more precisely, review should have caught this...

Attached is an hg export patch to change the UUID of nsIDOMNSDocument.idl; generated the UUID from uuidgen.exe.

Checked in the UUID change. That needs to go in on branch too if the main patch does. Thanks for posting the diff!

Is this the fix of Bug 475773?

Doesn't changing the UUID break compatibility? Aren't we trying to not do that from here on in?

Comment on attachment 356251 [details] [diff] [review] mozilla-central patch to add document.readyState property to documents If we take this on branch, we'll need to not change the interface, but rather add a new one for it, I think.