Closed
Bug 99181
Opened 23 years ago
Closed 23 years ago
Phase 1: Freeze some embedding APIs/interfaces...
Categories
(Core Graveyard :: Embedding: APIs, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.5
People
(Reporter: chak, Assigned: adamlock)
References
Details
Attachments
(4 files)
3.18 KB,
patch
|
Details | Diff | Splinter Review | |
3.54 KB,
patch
|
Details | Diff | Splinter Review | |
11.53 KB,
patch
|
Details | Diff | Splinter Review | |
34.19 KB,
patch
|
rpotts
:
superreview+
|
Details | Diff | Splinter Review |
NS_InitEmbedding NS_TermEmbedding nsIWebBrowser nsIContextMenuListener nsITooltipListener nsITooltipTextProvider nsIEmbeddingSiteWindow nsIWebBrowserSetup
Reporter | ||
Comment 1•23 years ago
|
||
Forgot to include this earlier, sorry. The above set of embedding interfaces/APIs have been identified as the ones we can freeze now with little fixup i.e. Phase 1 of the interface freeze process.
Chak can you look over these two interfaces to see I'm going the right away about them? One thing I noticed is Doxygen doesn't recognize the @status tag and passes it straight through. We should probably pick another way to specify an interface is frozen.
Updated•23 years ago
|
QA Contact: mdunn → depstein
Reporter | ||
Comment 5•23 years ago
|
||
Hi Jud : Could you please comment on the "@status" issue Adam's talking about?
Reporter | ||
Comment 6•23 years ago
|
||
Since we're following JavaDoc conventions here... only thing i noticed in the patches are the usage of "@retval" instead of "@return" as specified in http://java.sun.com/j2se/javadoc/writingdoccomments/index.html#tag Other than that r= chak on the two patches.
Comment 7•23 years ago
|
||
@status is it. if you want the full beef on how we arrived at it, checkout the comments in http://bugzilla.mozilla.org/show_bug.cgi?id=48726 . doxygen should adapt if it wants to do something special w/ @status.
Reporter | ||
Comment 8•23 years ago
|
||
Hi Adam: Another thing.... Per Jud - nsToolTipListener and nsITooltipTextProvider should be in different .idl files Thanks Chak
Reporter | ||
Updated•23 years ago
|
Target Milestone: --- → mozilla0.9.5
Reporter | ||
Comment 10•23 years ago
|
||
Setting 0.9.5 target milestone per Jud's comments.
Assignee | ||
Comment 11•23 years ago
|
||
Reporter | ||
Comment 12•23 years ago
|
||
r=chak Rick/Someone : Can one of you sr= this...Thanks
Comment 13•23 years ago
|
||
here are some comments about the big patch... 1. We need to decide how much HTML we should put inside of our doxygen comments... The HTML table describing nsIContextMenuListener::aContextFlag is unreadable to the naked eye :-( This is a decision that we 'can' pospone until the documentation group sweeps through these files...; 2. Did we finally decide to keep NS_DoIdleEmbeddingStuff(...) as a XP function? the last i heard, it was only *really* needed on the Mac... i don't know the answer here - i'm just asking :-) 3. Should we create a separate 'nsCTooltipTextProvider.h' file with #includes, contract-id and documentation describing the 'tooltip text provider' component that we expose... Right now, the component stuff is mixzed in with the nsITooltipTextProvider interface definition... 4. Can we remove the '#include nsIEnumeration.h' from nsIWebBrowserSetup.idl
Assignee | ||
Comment 14•23 years ago
|
||
Some answers: 1. HTML was the only way to do the table. Doxygen understands this limited markup and generates the correct equivalent for whatever output format you choose, rtf, html etc. Generally I've been extremely sparing with HTML markup but it's been used when it's the only or most obvious way to do something. 2. I've only marked NS_InitEmbedding & NS_TermEmbedding as FROZEN. Those other contentious functions are still up for review but I documented anyway while I was in there. I should probably put a @status tag in their documentation to say their under review. 3. Possibly, but since the only use for nsITooltipTextProvider is on a service registered with that contract ID I saw no point in seperating them. Do you think it's worth seperating it out? 4. Yes. So can I have an sr assuming I: 1. Mark the NS_DoIdleEmbeddingStuff and the other few functions as under review 2. Seperate out the contract ID if you think it should be done. 3. Remove #include "nsIEnumerator.idl" ?
Comment 15•23 years ago
|
||
hey adam, I think that it is useful to create a separate file for the 'Tooltip Text Provider' service... I didn't even realize that it was a service :-) If you could put the contract-id, the #include for nsITooltipTextProvider and the comments describing how the service is used into this new file... that would be GREAT!! Also, it looks like the mozilla license has changed!! So, we now need to use the new 'triple license' for ALL new files :-) take a look at http://www.mozilla.org/MPL/boilerplate-1.1/ And after *all* of this... sr=rpotts@netscape.com :-)
Comment 16•23 years ago
|
||
Comment on attachment 49753 [details] [diff] [review] All interfaces / APIs covered by this bug cleaned up and documented. Chak can you review please? sr=rpotts@netscape.com (after comments have been addressed)..
Attachment #49753 -
Flags: superreview+
Assignee | ||
Comment 17•23 years ago
|
||
Everything checked in, including Rick's suggestions. Chak, time for phase 2 whatever that might be!
Assignee | ||
Comment 18•23 years ago
|
||
Fixed
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 19•23 years ago
|
||
Checked patch checkins in 10/11 mozilla nightly build. Looks good.
Status: RESOLVED → VERIFIED
Updated•5 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•