Closed
Bug 540433
Opened 13 years ago
Closed 12 years ago
Remove nsIDocumentViewer
Categories
(Core :: General, defect)
Core
General
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: smaug, Assigned: Ms2ger)
References
Details
(Keywords: addon-compat, dev-doc-complete)
Attachments
(1 file)
62.39 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
I think we could merge nsIDocumentViewer and nsIContentViewer together. Most common case to use nsIDocumentViewer is to get nsIDocument from it, but nsIContentViewer has already attribute DOMDocument. For performance reasons I'd add [noscript,notxpcom] nsIDocumentPtr getDocument(); to nsIContenViewer and remove NS_IMETHOD GetDocument(nsIDocument** aResult) from nsIDocumentViewer. After that removing the remaining methods GetPresShell(nsIPresShell** aResult) GetPresContext(nsPresContext** aResult) should be easy. (Noticed this all while profiling nsDocShell::GetChannelIsUnsafe) Any objections? Suggestions?
![]() |
||
Comment 1•13 years ago
|
||
I'm all in favor of reducing the number of interfaces dealing with this stuff. I think the theory was that we could have non-document viewers (e.g. for plug-ins or images PDFs or whatnot), but I think that's a silly theory. We should just keep using synthetic documents.
Absolutely. Let's do it!
Assignee | ||
Updated•12 years ago
|
Assignee: Olli.Pettay → nobody
Hardware: x86 → All
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → Ms2ger
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #566602 -
Flags: review?(Olli.Pettay)
Assignee | ||
Comment 4•12 years ago
|
||
I should probably rename NS_NewDocumentViewer as well, and I'll rev the iid.
Reporter | ||
Comment 5•12 years ago
|
||
Comment on attachment 566602 [details] [diff] [review] Patch v1 >-NS_NewDocumentViewer(nsIDocumentViewer** aResult); >+NS_NewDocumentViewer(nsIContentViewer** aResult); Rename the method. > MAKE_CTOR(CreateNameSpaceManager, nsINameSpaceManager, NS_GetNameSpaceManager) >-MAKE_CTOR(CreateDocumentViewer, nsIDocumentViewer, NS_NewDocumentViewer) >+MAKE_CTOR(CreateDocumentViewer, nsIContentViewer, NS_NewDocumentViewer) CreateContentViewer ... NS_NewContentViewer > if (mIsCreatingPrintPreview) { >- nsCOMPtr<nsIDocumentViewer> dv = do_QueryInterface(mDocViewerPrint); >- if (dv) { >- parentView = dv->FindContainerView(); >+ nsCOMPtr<nsIContentViewer> cv = do_QueryInterface(mDocViewerPrint); >+ if (cv) { >+ parentView = cv->FindContainerView(); > } Could you file a followup to rename mDocViewerPrint and nsIDocumentViewerPrint (if I haven't filed that already)
Attachment #566602 -
Flags: review?(Olli.Pettay) → review+
Assignee | ||
Comment 6•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e915987a1cda
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Assignee | ||
Updated•12 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Updated•12 years ago
|
Keywords: addon-compat
Comment 7•12 years ago
|
||
The fix has been checked in by http://hg.mozilla.org/mozilla-central/rev/e915987a1cda However nsIDocumentViewer.h is required by following files in comm-central. /mailnews/base/src/nsMsgPrintEngine.cpp /mailnews/base/src/nsMsgStatusFeedback.cpp
Status: RESOLVED → REOPENED
Depends on: 694749
Resolution: FIXED → ---
Target Milestone: mozilla10 → ---
Assignee | ||
Updated•12 years ago
|
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Comment 8•11 years ago
|
||
This is mentioned on Firefox 10 for developers. In addition, I've added this new article: https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIContentViewer Neither nsIContentViewer nor nsIDocumentViewer had documentation previously. That article is not complete but is a start. If anyone wants to fill out the rest, awesome. But for the purposes of this particular bug, it's done.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•