No one ever assigns to it in JS anyway. MozReview-Commit-ID: EAoOXSFnwtl

MozReview-Commit-ID: FoMoVgCngGR

MozReview-Commit-ID: GsTN3kZATz7

MozReview-Commit-ID: 2FUstFDF2lF

MozReview-Commit-ID: A0GRXYLsupg

MozReview-Commit-ID: A4Me0H1MzL6

MozReview-Commit-ID: CnfelWtz0mT

MozReview-Commit-ID: IzjmFqySBpB

MozReview-Commit-ID: 53JPEy7AQtp

MozReview-Commit-ID: IBToVxx4bSs

MozReview-Commit-ID: 9ABdbYKZI6k

MozReview-Commit-ID: DAXrxIxiac4

MozReview-Commit-ID: EaUjTLeaQ0n

MozReview-Commit-ID: CCYEpeZTMLG

Comment on attachment 8947053 [details] [diff] [review] part 2. Stop using nsIContentViewer::GetDOMDocument in C++ Review of attachment 8947053 [details] [diff] [review]: ----------------------------------------------------------------- R=me but you have trailing whitespace in nsDocShell.cpp which you should clean up :-)

> R=me but you have trailing whitespace in nsDocShell.cpp which you should clean up :-) Done.

Comment on attachment 8947058 [details] [diff] [review] part 7. Remove nsIDOMDocument::GetElementsBy* methods Review of attachment 8947058 [details] [diff] [review]: ----------------------------------------------------------------- ::: editor/libeditor/TextEditorTest.cpp @@ +137,3 @@ > TEST_RESULT(rv); > + TEST_POINTER(domDoc.get()); > + nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc); Local style here seems to shove `doc` up right next to > (:-S). Please either change domDoc to have a space between > and domDoc, or make the other decls consistent. @@ +137,4 @@ > TEST_RESULT(rv); > + TEST_POINTER(domDoc.get()); > + nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc); > + TEST_POINTER(doc.get()); nit: trailing whitespace @@ +146,1 @@ > NS_ASSERTION(0!=count, "there are no text nodes in the document!"); nit: While you're in here please add spaces around this operator. ::: xpfe/appshell/nsWebShellWindow.cpp @@ +516,5 @@ > > // Find the menubar tag (if there is more than one, we ignore all but > // the first). > + nsCOMPtr<nsINodeList> menubarElements = > + aDoc->GetElementsByTagNameNS(NS_LITERAL_STRING("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"), nit: Is this URI not in a constant somewhere? @@ +650,5 @@ > /////////////////////////////// > nsCOMPtr<nsIContentViewer> cv; > mDocShell->GetContentViewer(getter_AddRefs(cv)); > if (cv) { > + nsCOMPtr<nsIDocument> menubarDOMDoc = cv->GetDocument(); nit: Remove the DOM from DOMDoc here - this isn't a DOMDoc anymore.

Comment on attachment 8947059 [details] [diff] [review] part 8. Remove nsIDOMDocument::GetElementById Review of attachment 8947059 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/xul/nsXULElement.cpp @@ +559,2 @@ > } > + nit: trailing whitespace. @@ +563,5 @@ > + nsCOMPtr<nsIDocument> document = content->GetUncomposedDoc(); > + if (!document) { > + return false; > + } > + nit: trailing whitespace ::: widget/cocoa/nsMenuBarX.mm @@ +483,3 @@ > return 0; > > NS_ConvertASCIItoUTF16 shortcutIDStr((const char *)shortcutID); I don't understand why this cast is happening - shortcutID already has the correct type?

Comment on attachment 8947061 [details] [diff] [review] part 10. Remove nsIDOMDocument's title attribute Review of attachment 8947061 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/html/MediaDocument.cpp @@ +405,5 @@ > > // set it on the document > if (aStatus.IsEmpty()) { > + IgnoredErrorResult ignored; > + SetTitle(title, ignored); Idle thought while reading these patches: We have this IgnoredErrorResult pattern a decent amount and it seems like there might be a cleaner way to do it without adding extra lines. e.g. class IgnoreRV { public: operator ErrorResult&() && { return mInner; } private: IgnoredErrorResult mInner; }; This type will implicitly coerce when used as a temporary into an ErrorResult&, meaning that you can ignore the result by calling it like: SetTitle(title, IgnoreRV()); Not sure if that's actually cleaner.

Comment on attachment 8947064 [details] [diff] [review] part 13. Remove nsIDOMDocument::CreateEvent Review of attachment 8947064 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/xul/tree/nsTreeBodyFrame.cpp @@ +4662,5 @@ > nsCOMPtr<nsIContent> content(GetBaseElement()); > if (!content) > return; > > + nsCOMPtr<nsIDocument> doc = content->OwnerDoc(); Am I correct to presume that OwnerDoc can never be null here because you removed the null check? Perhaps assert it just in case.

Comment on attachment 8947065 [details] [diff] [review] part 14. Remove various unused nsIDOMDocument bits Review of attachment 8947065 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/interfaces/core/nsIDOMDocument.idl @@ +8,3 @@ > /** > * The nsIDOMDocument interface represents the entire HTML or XML document. > * Conceptually, it is the root of the document tree, and provides the I'd ask you to ditch this trailing whitespace, but I have a strange feeling that the entire nsIDOMDocument interface is going away soon so may as well do it then.

> Local style here seems to shove `doc` up right next to > Yeah, local style is ... don't get me started. I guess I can follow it. > nit: trailing whitespace Fixed. > nit: While you're in here please add spaces around this operator. But local style is to economize on whitespace! ;) Seriously, fixed. > nit: Is this URI not in a constant somewhere? Not really. Most C++ code uses namespace ids instead. Though we do have nsGkAtoms::nsuri_xul which I suppose I could use here with nsDependentAtomString if you like. > nit: Remove the DOM from DOMDoc here - this isn't a DOMDoc anymore. Done.

> nit: trailing whitespace. > nit: trailing whitespace Fixed. >I don't understand why this cast is happening - shortcutID already has the >correct type? It used to take "char*" until https://hg.mozilla.org/mozilla-central/rev/1b66f53dcbdf3ad5ffd4d5cd851306173848ada8#l5.12 happened. The cast should have gone away then... I can remove it now. Done. > class IgnoreRV Hmm. I'll think on this. > Am I correct to presume that OwnerDoc can never be null here OwnerDoc can never be null, period. Not even after CC unlink, if I recall correctly. I can certainly add an assert. > but I have a strange feeling that the entire nsIDOMDocument interface is going away soon Well, all the members are. The comment in question is not, for now. I've removed the whitespace; rolled it into the last diff in this series.

> rolled it into the last diff in this series. Which is what your review comment is on! OK. Removing this IDL altogether will take a bit more work; we have 500-600 uses of nsIDOMDocument left in our tree. ;)

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2e2d6c02f40c part 1. Make nsIContentViewer's DOMDocument readonly. r=mystor https://hg.mozilla.org/integration/mozilla-inbound/rev/fe5144211ad5 part 2. Stop using nsIContentViewer::GetDOMDocument in C++. r=mystor https://hg.mozilla.org/integration/mozilla-inbound/rev/56635598e1e3 part 3. Remove nsIDOMDocument's doctype attribute. r=mystor https://hg.mozilla.org/integration/mozilla-inbound/rev/cd3e04508b23 part 4. Remove nsIDOMDocument's documentElement attribute. r=mystor https://hg.mozilla.org/integration/mozilla-inbound/rev/efa1210cb422 part 5. Remove some unused nsIDOMDocument node-creation APIs. r=mystor https://hg.mozilla.org/integration/mozilla-inbound/rev/63b985d018cf part 6. Remove nsIDOMDocument::CreateDocumentFragment. r=mystor https://hg.mozilla.org/integration/mozilla-inbound/rev/6452a9d56cf4 part 7. Remove nsIDOMDocument::GetElementsBy* methods. r=mystor https://hg.mozilla.org/integration/mozilla-inbound/rev/d29a865abef7 part 8. Remove nsIDOMDocument::GetElementById. r=mystor https://hg.mozilla.org/integration/mozilla-inbound/rev/d9527fdcd63e part 9. Remove nsIDOMDocument::CreateTreeWalker. r=mystor https://hg.mozilla.org/integration/mozilla-inbound/rev/4300f4eacf84 part 10. Remove nsIDOMDocument's title attribute. r=mystor https://hg.mozilla.org/integration/mozilla-inbound/rev/95f9f7eaff90 part 11. Remove nsIDOMDocument's stylesheet set APIs. r=mystor https://hg.mozilla.org/integration/mozilla-inbound/rev/0f7cbca8f63c part 12. Remove nsIDOMDocument's contentType attribute. r=mystor https://hg.mozilla.org/integration/mozilla-inbound/rev/d6abc2e5e842 part 13. Remove nsIDOMDocument::CreateEvent. r=mystor https://hg.mozilla.org/integration/mozilla-inbound/rev/45af32f305bd part 14. Remove various unused nsIDOMDocument bits. r=mystor

https://hg.mozilla.org/mozilla-central/rev/2e2d6c02f40c https://hg.mozilla.org/mozilla-central/rev/fe5144211ad5 https://hg.mozilla.org/mozilla-central/rev/56635598e1e3 https://hg.mozilla.org/mozilla-central/rev/cd3e04508b23 https://hg.mozilla.org/mozilla-central/rev/efa1210cb422 https://hg.mozilla.org/mozilla-central/rev/63b985d018cf https://hg.mozilla.org/mozilla-central/rev/6452a9d56cf4 https://hg.mozilla.org/mozilla-central/rev/d29a865abef7 https://hg.mozilla.org/mozilla-central/rev/d9527fdcd63e https://hg.mozilla.org/mozilla-central/rev/4300f4eacf84 https://hg.mozilla.org/mozilla-central/rev/95f9f7eaff90 https://hg.mozilla.org/mozilla-central/rev/0f7cbca8f63c https://hg.mozilla.org/mozilla-central/rev/d6abc2e5e842 https://hg.mozilla.org/mozilla-central/rev/45af32f305bd