99627 - Freeze nsIURIContentListener

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Chak Nanga]

Freeze nsIURIContentListener

Please refer to
http://www.mozilla.org/projects/embedding/EmbedInterfaceFreeze.html for the
issues to be addressed, if any, for this interface.

Please follow the guidelines outlined in "How to mark an interface as frozen?"
section of the document above.

Comment 1

[Chak Nanga]

->0.9.6

Comment 2

[David Epstein]

changing qa contact to Ashish.

Comment 3

[rpotts (gone)]

Attachment: patch to remove nsIURIContentListener::nsURILoadCommand flags

Comment 4

[rpotts (gone)]

The above patch does a couple of things:

1. It removes the nsURILoadCommand type.  This is replaced by a boolean
indicating whether the document should be loaded as 'preferred content' (ie.
because of a link click) or not.

2. nsIURIContentListener::IsPreferred() no longer takes a flag.  It is implied
by the name that the content is 'preferred' :-)

Comment 5

[Judson Valeski]

r=valeski assuming the following have been checked:

mailto: links in the browser
mailto: links in the mail news content area
URLs in aim windows
Open in new window (right click) over a link

Comment 6

[Scott MacGregor]

Comment on attachment 54111 [details] [diff] [review]
patch to remove nsIURIContentListener::nsURILoadCommand flags

sr=mscott assuming (in addition to judson's excellent suggestions), you've tried:
1) reading mail messages
2) clicking on http urls inside of mail messages and making sure they end up in a new window
3) clicking on a link requiring a helper app

Comment 7

[rpotts (gone)]

patch checked in...

Comment 8

[rpotts (gone)]

Attachment: patch to the security code...

Comment 9

[hacker formerly known as [e-mail]]

security patch has been checked in.

Comment 10

[timeless]

Thank you guys very much

$ gmake
/usr/bin/perl -I/home/timeless/mozilla/config /home/timeless/mozilla/config/build-list.pl ../../../../config/final-link-
libs phembedmoz
../../../../config/nsinstall -R -m 644 /home/timeless/mozilla/embedding/browser/photon/src/PtMozilla.h /home/timeless/mo
zilla/embedding/browser/photon/src/PhMozEmbedStream.h /home/timeless/mozilla/embedding/browser/photon/src/stdhdrs.h ../.
./../../dist/include/phembedmoz
/usr/bin/perl -I/home/timeless/mozilla/config /home/timeless/mozilla/config/build-list.pl ../../../../dist/include/phemb
edmoz/.headerlist PtMozilla.h PhMozEmbedStream.h stdhdrs.h
WebBrowserContainer.cpp
Building deps for /home/timeless/mozilla/embedding/browser/photon/src/WebBrowserContainer.cpp
g++ -o WebBrowserContainer.o -c -DOSTYPE=\"QNX6.00\" -DOSARCH=\"QNX\" -DOJI  -I../../../../dist/include/xpcom -I../../..
/../dist/include/string -I../../../../dist/include/chrome -I../../../../dist/include/docshell -I../../../../dist/include
/webbrwsr -I../../../../dist/include/dom -I../../../../dist/include/widget -I../../../../dist/include/necko -I../../../.
./dist/include/uriloader -I../../../../dist/include/cookie -I../../../../dist/include/intl -I../../../../dist/include/pr
ef -I../../../../dist/include/prefmigr -I../../../../dist/include/appshell -I../../../../dist/include/uconv -I../../../.
./dist/include/appcomps -I../../../../dist/include/windowwatcher -I../../../../dist/include/js -I../../../../dist/includ
e/gfx -I../../../../dist/include/view -I../../../../dist/include/content -I../../../../dist/include/webshell -I../../../
../dist/include/layout -I../../../../dist/include/exthandler -I../../../../dist/include/shistory -I../../../../dist/incl
ude/embed_base -I../../../../dist/include/find -I../../../../dist/include/mpfilelocprovider -I../../../../dist/include/h
elperAppDlg -I../../../../dist/include/mimetype -I../../../../dist/include/phembedmoz -I../../../../dist/include -I/home
/timeless/mozilla/obj-photon-x86-pc-nto-qnx/dist/include/nspr      -I/usr/X11R6/include     -I/usr/X11R6/include -fno-rt
ti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Wbad-function-cast -Wcast-align -Woverloaded-virtual -Wsynth -Wno
-long-long -pipe  -DNDEBUG -DTRIMMED  -I/usr/X11R6/include -DMOZILLA_CLIENT -Wp,-include -Wp,../../../../config-defs.h -
D_POSIX_C_SOURCE=199506 /home/timeless/mozilla/embedding/browser/photon/src/WebBrowserContainer.cpp
In file included from /home/timeless/mozilla/embedding/browser/photon/src/stdhdrs.h:45,
                 from /home/timeless/mozilla/embedding/browser/photon/src/WebBrowserContainer.h:44,
                 from /home/timeless/mozilla/embedding/browser/photon/src/WebBrowserContainer.cpp:40:
../../../../dist/include/xp_core.h:252: warning: `_UINT32' redefined
/usr/include/sys/target_nto.h:14: warning: this is the location of the previous definition
/home/timeless/mozilla/embedding/browser/photon/src/WebBrowserContainer.cpp:597: type specifier omitted for parameter
/home/timeless/mozilla/embedding/browser/photon/src/WebBrowserContainer.cpp:597: parse error before `,'
/home/timeless/mozilla/embedding/browser/photon/src/WebBrowserContainer.cpp:600: prototype for `nsresult CWebBrowserCont
ainer::IsPreferred(...)' does not match any in class `CWebBrowserContainer'
/home/timeless/mozilla/embedding/browser/photon/src/WebBrowserContainer.h:115: candidate is: nsresult CWebBrowserContain
er::IsPreferred(const char *, char **, PRBool *)
/home/timeless/mozilla/embedding/browser/photon/src/WebBrowserContainer.cpp: In method `nsresult CWebBrowserContainer::I
sPreferred(...)':
/home/timeless/mozilla/embedding/browser/photon/src/WebBrowserContainer.cpp:602: `aContentType' undeclared (first use th
is function)
/home/timeless/mozilla/embedding/browser/photon/src/WebBrowserContainer.cpp:602: (Each undeclared identifier is reported
 only once
/home/timeless/mozilla/embedding/browser/photon/src/WebBrowserContainer.cpp:602: for each function it appears in.)
/home/timeless/mozilla/embedding/browser/photon/src/WebBrowserContainer.cpp:614: `aCanHandleContent' undeclared (first u
se this function)
gmake: *** [WebBrowserContainer.o] Error 1
$ pwd
/home/timeless/mozilla/obj-photon-x86-pc-nto-qnx/embedding/browser/photon/src

Comment 11

[timeless]

bustage fix checked in, please be more careful next time.

Comment 12

[rpotts (gone)]

Well it appears that my last comment to this bug got lost :-(

as i said before, i do not believe that my check in *did* cause this bustage.  
if you look at rev 1.10 of WebBrowerContainer.cpp (the version that i checked 
in) you will notice that the signature for IsPreferred(...) is correct.

however the changes for rev 1.11 to that file made by 'dinglis' reverted the 
signature back to the incorrect one !!

Comment 13

[timeless]

ah. yeah i guess if you (or i) are going to touch ports, we should contact the
people who own the code, otherwise they'll assume nothing has changed and that
they can force commit or ignore merge issues. sorry to be accusatory.

Comment 14

[rpotts (gone)]

all that's left with this bug is fixing up the comments in nsIURIContentListener.idl

-- rick

Comment 15

[rpotts (gone)]

Attachment: new comments for nsIURIContentListener.idl

Comment 16

[rpotts (gone)]

hey adam and mscott,

can i get an r= and sr= on the new comments for nsIURIContentListener.idl...

thanks,
-- rick

Comment 17

[Adam Lock]

Comment on attachment 58655 [details] [diff] [review]
new comments for nsIURIContentListener.idl

Have you run it through doxygen? I'm worried that the use of fullstops such as
in the middle of @return might confuse it.

I also spotted a couple of issues:

1. @status should now be FROZEN.
2. Typo "listenersv"
3. Use of "I" and "we" as in "I really envision ..." & "URI we are going to try
and open."

Otherwise r=adamlock

Comment 18

[rpotts (gone)]

Attachment: another patch to nsIURIContentListener that addressed Adam's comments

Comment 19

[rpotts (gone)]

i've just checked in the comment changes to nsIURIContentListener.idl (except
for the FROZEN)...

I'll mark this interface as FROZEN as soon as i'm sure it looks good.

-- rick

Comment 20

[rpotts (gone)]

-> 0.9.7

Comment 21

[Scott MacGregor]

Comment on attachment 58760 [details] [diff] [review]
another patch to nsIURIContentListener that addressed Adam's comments

sr=mscott for posterity's sake. Your comments look good to me Rick.

Comment 22

[rpotts (gone)]

I believe that we're all happy with the structure of this interface...  So i'll
attach a patch that *just* marks this interface as @FROZEN.

Any objections?? anyone???

-- rick

Comment 23

[rpotts (gone)]

Attachment: marks interface as FROZEN

This patch *only* marks the interface as @status FROZEN.

Comment 24

[Chak Nanga]

Comment on attachment 78860 [details] [diff] [review]
marks interface as FROZEN

r=chak

Comment 25

[Scott MacGregor]

Comment on attachment 78860 [details] [diff] [review]
marks interface as FROZEN

sr=mscott

Comment 26

[Asa Dotzler [:asa]]

Comment on attachment 78860 [details] [diff] [review]
marks interface as FROZEN

a=asa (on behalf of drivers) for checkin to the 1.0 branch

Comment 27

[rpotts (gone)]

patch checked in to both trunk and 1.0 branch.

Comment 28

[Ashish Bhatt]

marking as verified