99181 - Phase 1: Freeze some embedding APIs/interfaces...

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Chak Nanga]

NS_InitEmbedding

NS_TermEmbedding

nsIWebBrowser

nsIContextMenuListener

nsITooltipListener 

nsITooltipTextProvider

nsIEmbeddingSiteWindow

nsIWebBrowserSetup

Comment 1

[Chak Nanga]

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.

Comment 2

[Adam Lock]

Attachment: nsIContextMenuListener - cleaned up

Comment 3

[Adam Lock]

Attachment: nsITooltipListener - cleaned up

Comment 4

[Adam Lock]

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.

Comment 5

[Chak Nanga]

Hi Jud : Could you please comment on the "@status" issue Adam's talking about?

Comment 6

[Chak Nanga]

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

[Judson Valeski]

@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.

Comment 8

[Chak Nanga]

Hi Adam:

Another thing....

Per Jud - nsToolTipListener and nsITooltipTextProvider should be in different
.idl files

Thanks
Chak

Comment 9

[Adam Lock]

Attachment: Patch incorporates suggestions, splits nsITooltipTextProvider into its own idl file

Comment 10

[Chak Nanga]

Setting 0.9.5 target milestone per Jud's comments.

Comment 11

[Adam Lock]

Attachment: All interfaces / APIs covered by this bug cleaned up and documented. Chak can you review please?

Comment 12

[Chak Nanga]

r=chak

Rick/Someone : Can one of you sr= this...Thanks

Comment 13

[rpotts (gone)]

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

Comment 14

[Adam Lock]

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

[rpotts (gone)]

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=[e-mail] :-)

Comment 16

[rpotts (gone)]

Comment on attachment 49753 [details] [diff] [review]
All interfaces / APIs covered by this bug cleaned up and documented. Chak can you review please?

sr=[e-mail]
(after comments have been addressed)..

Comment 17

[Adam Lock]

Everything checked in, including Rick's suggestions.

Chak, time for phase 2 whatever that might be!

Comment 18

[Adam Lock]

Fixed

Comment 19

[David Epstein]

Checked patch checkins in 10/11 mozilla nightly build. Looks good.