Last Comment Bug 299054 - Mozilla Plug-In API Proposal: Plugin able to Read Response Headers
: Mozilla Plug-In API Proposal: Plugin able to Read Response Headers
Status: RESOLVED FIXED
[need testcase]
: dev-doc-complete, fixed1.8.0.12, fixed1.8.1.4
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
:
: Benjamin Smedberg [:bsmedberg]
Mentors:
Depends on:
Blocks: 376201
  Show dependency treegraph
 
Reported: 2005-06-28 14:08 PDT by Edwin Wong
Modified: 2008-03-07 14:47 PST (History)
14 users (show)
dveditz: blocking1.8.1.4+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed fix (12.54 KB, patch)
2007-03-06 10:57 PST, Deneb Meketa
jst: review-
Details | Diff | Splinter Review
Proposed fix, take 2 (14.28 KB, patch)
2007-03-15 20:56 PDT, Deneb Meketa
jaas: review+
jst: superreview+
Details | Diff | Splinter Review
Trunk version of Denebs patch. (9.50 KB, patch)
2007-03-16 14:22 PDT, Johnny Stenback (:jst, jst@mozilla.com)
dmeketa: review+
Details | Diff | Splinter Review
Proposed fix for branch (14.96 KB, patch)
2007-03-16 17:38 PDT, Deneb Meketa
jst: review+
dveditz: approval1.8.1.4+
dveditz: approval1.8.0.12+
Details | Diff | Splinter Review

Description Edwin Wong 2005-06-28 14:08:03 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/412 (KHTML, like Gecko) Safari/412
Build Identifier: Mozilla/5.0

The Flash Player would like to be able to read HTTP response headers. This is currently impossible.

We propose a new member at the end of NPStream:
 #define NPVERS_STREAM_HEADERS xx

 typedef struct _NPStream
 {
   void* pdata;
   void* ndata;
   const char* url;
   uint32 end;
   uint32 lastmodified;
   void* notifyData;
   const char* headers;
 } NPStream;


 /* headers is present if the browser minor version returned by NPN_Version
  * is >= NPVERS_STREAM_HEADERS. It contains a verbatim copy of all headers
  * received in the URL response. In the case of HTTP responses, the HTTP
  * status line is included at the beginning. Header lines are separated
  * by \n characters (NOT \r\n). The headers buffer is terminated by \n\0
  * (NOT \n\n\0). */

Reproducible: Always

Steps to Reproduce:
1. use flash or other plugin to read response header
2.
3.

Actual Results:  
unable to read

Expected Results:  
should be able to read response header
Comment 1 Jeff Mott 2005-06-29 11:35:16 PDT
Submitted to plugin futures last year.  See
http://wiki.mozilla.org/uploads/PluginFutures/response-headers.html
Comment 2 Deneb Meketa 2007-03-06 10:57:08 PST
Created attachment 257534 [details] [diff] [review]
Proposed fix

This follows a proposal made on plugin-futures (and already implemented by Opera), and modifies npapi.h accordingly (also adding some missing NPVERS_HAS_* constants). This is working locally for me in quick tests against the Flash Player on Windows. This is my first attempt to contribute to Mozilla, so I'm sure I've probably done something dumb, but I've tried to follow neighboring code and documented practices as much as I was able to figure out. We (Adobe) would ideally like this incorporated into a Firefox release as soon as is practical, but I confess ignorance of upcoming milestones and the standards for inclusion in them. Thanks in advance to Johnny and whoever else touches this.
Comment 3 Christian :Biesinger (don't email me, ping me on IRC) 2007-03-13 08:03:30 PDT
Shouldn't you include the version too, so that the status line is something like HTTP/1.1 200 OK? (you can get that from nsIHttpChannelInternal)

You need to generate a new UUID when changing an interface.

+        sprintf(codeStr, "%lu", statusNum);

You could use status.AppendInt here
Comment 4 Johnny Stenback (:jst, jst@mozilla.com) 2007-03-14 14:00:53 PDT
Comment on attachment 257534 [details] [diff] [review]
Proposed fix

- In base/public/npapi.h:

-#define NP_VERSION_MINOR 16
+#define NP_VERSION_MINOR 17

We need to figure out how to deal with this vs the NPObject enumeration support that's already on the Mozilla trunk, which already bumped the version number to 17. There's discussion about that on the plugin-futures list...

- In base/public/nsIHTTPHeaderListener.idl:

 [scriptable, uuid(8b246748-1dd2-11b2-9512-9dc84a95fc2f)]
 interface nsIHTTPHeaderListener : nsISupports
 {
   /**
+   * Called once for the HTTP Response status line.
+   * Value does NOT include a terminating newline.
+   * NOTE: You must copy this value.
+   */
+  void statusLine(in string line);
+
+  /**
    * Called for each HTTP Response header.
    * NOTE: You must copy the values of the params.  
    */
   void newResponseHeader(in string headerName, in string headerValue);
 };

As biesi already pointed out, you need to generate a new uuid for this interface now that you're changing it, and also, we tend to add new methods at the end of interfaces rather than in the beginning, for some additional (though often questionable) protection against compatibility. And generally, when we do this on the branch we create a new interface instead of changing an existing one. I can do that part for you once this patch is ready to land etc if you want.

- In ns4xPluginStreamListener::OnStartBinding():

+    mResponseHeaderBuf = PL_strdup(PromiseFlatCString(mResponseHeaders).get());

No need for the PromiseFlatCString() here, mResponseHeaders is already a flat string, so yo ushould be able to use .get() directly on it.

+    mNPStream.headers = mResponseHeaderBuf;

I *think* we don't need to do this, we should be able to make mNPStream.headers point directly into the internal buffer in mResponseHeaders (.get()), but then again, I don't know absolutely for sure that the API guarantees that we don't change mResponseHeaders after we do this, which would be bad. So to play it safe we should probably leave this as is.

- In ns4xPluginStreamListener::StatusLine():

+  mResponseHeaders.Append("\n");

Use .Append('\n') here, since you're appending a single character.

- In ns4xPluginStreamListener::NewResponseHeader():

+  mResponseHeaders.Append("\n");

Ditto.

- In base/src/nsPluginHostImpl.cpp:

+      nsCString status;
+      status.Append("HTTP ");

These two lines could be moved into the following if check.

+      PRUint32 statusNum;
+      if (NS_SUCCEEDED(httpChannel->GetResponseStatus(&statusNum)) && statusNum < 1000)
+      {
+        char codeStr[10];
+        sprintf(codeStr, "%lu", statusNum);
+        status.Append(codeStr);

As biesi mentioned, use AppendInt() here instead.

+        status.Append(" ");

status.Append(' ');

+        nsCString statusText;
+        if (NS_SUCCEEDED(httpChannel->GetResponseStatusText(statusText)))
+        {
+          status.Append(statusText);
+          listener->StatusLine(PromiseFlatCString(status).get());

Shouldn't need PromiseFlatCString() here.

Other than those minor issues this looks good. r- until those get addressed. And again, let me know if you need any help here in any way.
Comment 5 Deneb Meketa 2007-03-15 17:07:30 PDT
(In reply to comment #3)

> Shouldn't you include the version too, so that the status line is
> something like HTTP/1.1 200 OK? (you can get that from nsIHttpChannelInternal)

Yes, thanks, I'll do that.  I'm a little nervous about the comment "be aware that this interface will change and you will be broken", but I'm hopeful that the plugin code will always travel with the larger codebase, meaning that any breaks will quickly be noticed and fixed.

(In reply to comment #4)

> We need to figure out how to deal with this vs the NPObject enumeration support
> that's already on the Mozilla trunk, which already bumped the version number to
> 17. There's discussion about that on the plugin-futures list...

Yep.  For now, I'm going to be lazily optimistic that the proposal to re-number enumeration as 18, and leave headers as 17, will work.  Since no products have ever shipped with enumeration enabled, that seems OK in theory.  I will of course be ready to switch headers over to 18 if the need arises.  Johnny, if you feel that plugin-futures has reached consensus on this issue, would you be willing to re-number enumeration as part of landing this patch?  I think it might not require any actual work other than adding

  #define NPVERS_HAS_NPRUNTIME_ENUMERATION 18

> As biesi already pointed out, you need to generate a new uuid for this
> interface now that you're changing it, and also, we tend to add new methods at
> the end of interfaces rather than in the beginning, for some additional (though
> often questionable) protection against compatibility. And generally, when we do
> this on the branch we create a new interface instead of changing an existing
> one. I can do that part for you once this patch is ready to land etc if you
> want.

Cool, thanks.  I wasn't sure what the policy was for UUIDs; I'll make a new one.  Adding only to the end is familiar, I should have thought of that and will do so.  As far as branches and creating new interfaces, I'm happy to do that if I can learn how it all works, or I'm also happy to let Johnny do it if that's more expiditious.  I don't actually know what the branch structure is, but I assume whoever lands the patch takes care of that.  Should I just create a new interface in my patch, and let that be the solution for both trunk and branch, or is there value in reducing the number of interfaces in the trunk code?  And if I create a new interface, what should I name it - nsIHTTPHeaderListener2?

> No need for the PromiseFlatCString() here, mResponseHeaders is already a flat
> string, so yo ushould be able to use .get() directly on it.

Are you sure?  I carefully read the XPCOM string guide, and it seems quite clear in asserting that, when you need a raw character pointer, you should use PromiseFlatCString, and a runtime check will ensure that no unnecessary work is done.  The string in question (mResponseHeaders) has been built up using Append, and after about 30 minutes of browsing the XPCOM string implementation, I still haven't managed to convince myself that an nsCString is guaranteed to be flat after multiple calls to Append.  I have no experience in this code, so if you say that I shouldn't use PromiseFlatCString here, then I won't, but then I'd think that the XPCOM string guide might need to be updated, because I thought I was following what it suggested.  Maybe I'm just being thick about something?

> I *think* we don't need to do this, we should be able to make mNPStream.headers
> point directly into the internal buffer in mResponseHeaders (.get()), but then
> again, I don't know absolutely for sure that the API guarantees that we don't
> change mResponseHeaders after we do this, which would be bad. So to play it
> safe we should probably leave this as is.

Yes, that was definitely my thinking.  I'm not trying to hyper-optimize this code, since the number of streams that go to a plugin is typically small, and response times are dominated by network or filesystem delays.  And safety always seems like a good idea when dealing with third-party plugins.

> [Various references to Append(char), AppendInt, and conditional structure]

Will make these changes.  Still getting my feet wet in the string classes here.

Many thanks for all the input.  New patch coming soon.
Comment 6 Johnny Stenback (:jst, jst@mozilla.com) 2007-03-15 17:59:22 PDT
(In reply to comment #5)
> > Shouldn't you include the version too, so that the status line is
> > something like HTTP/1.1 200 OK? (you can get that from nsIHttpChannelInternal)
> 
> Yes, thanks, I'll do that.  I'm a little nervous about the comment "be aware
> that this interface will change and you will be broken", but I'm hopeful that
> the plugin code will always travel with the larger codebase, meaning that any
> breaks will quickly be noticed and fixed.

Yes, indeed. That comment is more for embedders of mozilla code that is out of our control, not for internal Mozilla code.

> (In reply to comment #4)
> 
> > We need to figure out how to deal with this vs the NPObject enumeration support
> > that's already on the Mozilla trunk, which already bumped the version number to
> > 17. There's discussion about that on the plugin-futures list...
> 
> Yep.  For now, I'm going to be lazily optimistic that the proposal to re-number
> enumeration as 18, and leave headers as 17, will work.  Since no products have
> ever shipped with enumeration enabled, that seems OK in theory.  I will of
> course be ready to switch headers over to 18 if the need arises.  Johnny, if
> you feel that plugin-futures has reached consensus on this issue, would you be
> willing to re-number enumeration as part of landing this patch?

Yes, I can take care of that part for sure. And it is looking like we'll simply flip that version number meaning on the trunk.

> Cool, thanks.  I wasn't sure what the policy was for UUIDs; I'll make a new
> one.  Adding only to the end is familiar, I should have thought of that and
> will do so.  As far as branches and creating new interfaces, I'm happy to do
> that if I can learn how it all works, or I'm also happy to let Johnny do it if
> that's more expiditious.  I don't actually know what the branch structure is,
> but I assume whoever lands the patch takes care of that.  Should I just create
> a new interface in my patch, and let that be the solution for both trunk and
> branch, or is there value in reducing the number of interfaces in the trunk
> code?  And if I create a new interface, what should I name it -
> nsIHTTPHeaderListener2?

There's some overhead in having more interfaces, so on the trunk we'll want what you did in this patch (with the addition at the end and a new UUID), but for the branch we'll want to add a new interface, and traditionally when we've done that for branches in the past we put the branch name at the end of the interface, i.e. in this case that would be nsIHTTPHeaderListener_MOZILLA_1_8_BRANCH for this addition. And this new interface could inherit from nsIHTTPHeaderListener. Feel free to do that if you're working on a new patch, if not, I'm happy to do that for you.

> > No need for the PromiseFlatCString() here, mResponseHeaders is already a flat
> > string, so yo ushould be able to use .get() directly on it.
> 
> Are you sure?  I carefully read the XPCOM string guide, and it seems quite
> clear in asserting that, when you need a raw character pointer, you should use
> PromiseFlatCString, and a runtime check will ensure that no unnecessary work is
> done.  The string in question (mResponseHeaders) has been built up using
> Append, and after about 30 minutes of browsing the XPCOM string implementation,
> I still haven't managed to convince myself that an nsCString is guaranteed to
> be flat after multiple calls to Append.  I have no experience in this code, so
> if you say that I shouldn't use PromiseFlatCString here, then I won't, but then

Generally the string guide is right, and there's very little overhead in using PromiseFlatCString() on an nsCString, but since you know it's an nsCString we know it's a single fragment string (it's a nsTString_CharT, which in the code is defined to be flat, and null terminating). So yes, I am sure :)

> I'd think that the XPCOM string guide might need to be updated, because I
> thought I was following what it suggested.  Maybe I'm just being thick about
> something?

That may be, I haven't read that in a long time myself, but PromiseFlat[C]String() is still necessary with some other types of string objects, notably nsAString, which is a more abstract string class, and it is *not* guaranteed to be flat nor null teminated.

[...]
> Many thanks for all the input.  New patch coming soon.

Great, thanks!

When you've got a new patch, please request review from joshmoz@gmail.com and sr from me, and we'll get this rolled in!
Comment 7 Deneb Meketa 2007-03-15 20:56:05 PDT
Created attachment 258751 [details] [diff] [review]
Proposed fix, take 2

New patch, hopefully final, incorporating review comments.

I rewrote my code in nsPluginStreamListenerPeer::SetUpStreamListener.  After adding HTTP version to the status line, the original Append style was getting rather verbose, so I switched to using nsPrintfCString to make the code more compact.  I now understand that ns[C]String is always flat, and that PromiseFlat[C]String is only needed when working with nsA[C]String.  The XPCOM string guide does in fact say that; I just hadn't read it enough times yet :)

Since my patch is based on the MOZILLA_1_8_BRANCH code, I decided to define a new interface, as Johnny suggested.  I stuck my interface definition in the same file as the original - is that a no-no, should each .idl file have only one interface, or is this OK for a branch-only interface?  Feel free to re-jigger this arrangement if it's easy and will reduce the back-and-forth, although of course I'll be glad to fix things up myself if you'd prefer.  Also, I'm not sure what the plan is for landing this in both the trunk and the branch - should I make a second patch for the trunk, or is the person who lands this going to apply the existing patch to both, then manually collapse the extended interface back down to its original name in the case of the trunk?

Thanks again all.
Comment 8 Josh Aas 2007-03-16 11:52:06 PDT
Comment on attachment 258751 [details] [diff] [review]
Proposed fix, take 2

+#define NP_VERSION_MINOR 17

From the plugin-futures mailing list this looks correct, but I'd like to see final sign-off on that decision from jst in this bug before this patch lands.

+  if (!mResponseHeaders.IsEmpty())
+  {
+    mResponseHeaderBuf = PL_strdup(mResponseHeaders.get());
+    mNPStream.headers = mResponseHeaderBuf;
+  }

I know there are examples to the contrary in the file, but for any new code please put the opening curly brace of the if statement on the same line as the test. Same comment for all of the places this happens in your patch.

+  mResponseHeaders.Append(headerName);

I'm guessing Append(...) does a null check for you here, but I'm not sure myself.

We'll also need a trunk version of this patch, I'd rather not land things on the branch until they are already on the trunk.

r+ because none of my comments necessarily require a respin of the patch, lets just make sure they are addressed before checkin.

Thanks!
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2007-03-16 13:56:29 PDT
(In reply to comment #7)
[...]
> compact.  I now understand that ns[C]String is always flat, and that
> PromiseFlat[C]String is only needed when working with nsA[C]String.  The XPCOM
> string guide does in fact say that; I just hadn't read it enough times yet :)

Ok :)

> Since my patch is based on the MOZILLA_1_8_BRANCH code, I decided to define a
> new interface, as Johnny suggested.  I stuck my interface definition in the
> same file as the original - is that a no-no, should each .idl file have only
> one interface, or is this OK for a branch-only interface?  Feel free to

That's fine, and is even the norm in these situations.

[...]
> I'm not sure what the plan is for landing this in both the trunk and the branch
> - should I make a second patch for the trunk, or is the person who lands this
> going to apply the existing patch to both, then manually collapse the extended
> interface back down to its original name in the case of the trunk?

I'll port this to the trunk and attach it, and have you look it over. I've already got a tree where I can do that, so it's no big deal here.

The way this will get into the tree is that we'll land it on the trunk first, then we'll request approval to get this landed on the branch, and once the branch is ready and taking changes again we'll be able to land this change there. All this happens in the bug, so keep your eyes on this bug and you'll know when it goes in, trunk and branch.

> Thanks again all.

Thank you!
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2007-03-16 14:22:12 PDT
Created attachment 258841 [details] [diff] [review]
Trunk version of Denebs patch.

Deneb, does this look right to you? It's the same patch w/o the new interface, and with the formatting things Josh mentioned and a few others addressed, and a few other minor tweaks to match the trunk version of this code closer. And this of course bumps up the version number for the NPObject enumeration support.
Comment 11 Johnny Stenback (:jst, jst@mozilla.com) 2007-03-16 14:24:46 PDT
Comment on attachment 258751 [details] [diff] [review]
Proposed fix, take 2

sr=jst, and yeah, the NPObject version bump has been cleared with eveyone involved AFAIK.
Comment 12 Christian :Biesinger (don't email me, ping me on IRC) 2007-03-16 14:42:17 PDT
+        nsPrintfCString status("HTTP%s %lu %s", ver.get(), statusNum,
+                               statusText.get());

shouldn't this pass a longer length to nsPrintfCString than the default of 15? (also, is %lu really correct for a 32-bit integer? On 64-bit unix, a long is 64-bit...)
Comment 13 Johnny Stenback (:jst, jst@mozilla.com) 2007-03-16 14:55:54 PDT
(In reply to comment #12)
> +        nsPrintfCString status("HTTP%s %lu %s", ver.get(), statusNum,
> +                               statusText.get());
> 
> shouldn't this pass a longer length to nsPrintfCString than the default of 15?

Yes, good catch! Should we hard code a number, or attempt to calculate it from statusText's length etc?

> (also, is %lu really correct for a 32-bit integer? On 64-bit unix, a long is
> 64-bit...)

If http://lxr.mozilla.org/mozilla/source/nsprpub/pr/include/prprf.h#49 is correct, the %lu is 32-bit. But we're guaranteed that the numbers we're dealing with are small enough to even fit in 16-bits aren't we? So we could even use %u here, and that ought to work too.
Comment 14 Johnny Stenback (:jst, jst@mozilla.com) 2007-03-16 15:00:50 PDT
http://lxr.mozilla.org/mozilla/source/nsprpub/pr/src/io/prprf.c#531

%lu etc *is* 32-bit when using nspr's printf n' friends.
Comment 15 Deneb Meketa 2007-03-16 15:20:38 PDT
Oh, that's so horrible that nsPrintfCString truncates at 15 chars unless you manually pre-compute the length.  Talk about a surprising API.  Thanks for catching that.  I think hard-coding 100 chars is fine; in the unlikely event that we truncate the status text, that's no big deal.

I am glad to hear that %lu is reliably 32-bit.  In glibc, the behavior is not documented, but I believe it is the same, with %Lu being reliably 64-bit.  Whoever invented C's flexibly-sized types should be shot.

I am in the midst of building the trunk with Johnny's patch and verifying that my plugin can see the headers with that build.  Will report back soon.

Thanks again.
Comment 16 Deneb Meketa 2007-03-16 15:24:59 PDT
BTW, an academic issue: it is important that %lu align with the actual size of PRUint32, even if a smaller size is adequate, because sprintf uses a varargs list, which will fail catastrophically if the wrong size arg is pulled off.  It seems that our %lu implementation does the right thing.
Comment 17 Christian :Biesinger (don't email me, ping me on IRC) 2007-03-16 15:44:42 PDT
(In reply to comment #14)
> %lu etc *is* 32-bit when using nspr's printf n' friends.

Ah sorry, you're right.

Daneb:
The printf man page from glibc says:

       l      (ell)  A  following integer conversion corresponds to a long int
              or unsigned long int argument [...]

So I kind of doubt that it's reliably 32-bit there. Anyway... that's somewhat offtopic here :)
Comment 18 Deneb Meketa 2007-03-16 16:38:53 PDT
Johnny's patch works great for me with the trunk code, so I would give it r+, except Bugzilla ain't letting me.  Thanks for the legwork.

Wanna know something funny and evil?  "HTTP/1.1 200 OK" is precisely 15 characters.  That bug was trying to hide.
Comment 19 Johnny Stenback (:jst, jst@mozilla.com) 2007-03-16 16:48:23 PDT
(In reply to comment #18)
> Johnny's patch works great for me with the trunk code, so I would give it r+,
> except Bugzilla ain't letting me.  Thanks for the legwork.

It should let you do that now :) Thanks for the patch!

> Wanna know something funny and evil?  "HTTP/1.1 200 OK" is precisely 15
> characters.  That bug was trying to hide.

Mmm, evil.
Comment 20 Johnny Stenback (:jst, jst@mozilla.com) 2007-03-16 17:00:20 PDT
Fix checked in on the trunk. Marking bug fixed.

Deneb, since you've probably got a branch tree there with this change in it, care to update to the final changes (the formatting changes Josh pointed out, the nsPrintfCString change) and attaching a new patch for the branch, and requesting approval1.8.1.4? and approvak1.8.0.12? on the patch?
Comment 21 Deneb Meketa 2007-03-16 17:38:06 PDT
Created attachment 258859 [details] [diff] [review]
Proposed fix for branch

New patch for branch, as requested.  This differs from "take 2" only in formatting and the nsPrintfCString size parameter.
Comment 22 Daniel Veditz [:dveditz] 2007-03-21 16:09:49 PDT
Comment on attachment 258859 [details] [diff] [review]
Proposed fix for branch

Johnny: did we address the trunk/branch version confusion with the extra Sun-Java added feature that's also called version 17? Need a new patch or is this one right?
Comment 23 Deneb Meketa 2007-03-21 16:42:55 PDT
I think we agreed to recharacterize NPObject enumeration as version 18, since nothing had ever shipped with it in.  If you look here, you'll see that Johnny landed the trunk patch with #define NPVERS_HAS_NPOBJECT_ENUM 18:

http://lxr.mozilla.org/seamonkey/source/modules/plugin/base/public/npapi.h

And the goal was to land this fix (reponse headers) on the branch before landing NPObject enumeration on the branch (if that's even ever done, it may just stay in the trunk and arrive on whatever the next branch is).
Comment 24 Christian :Biesinger (don't email me, ping me on IRC) 2007-03-21 16:55:38 PDT
hm, I'm late with this comment, but... the !NS_SUCCEEDED in the patch should be NS_FAILED instead
Comment 25 Johnny Stenback (:jst, jst@mozilla.com) 2007-03-23 13:33:13 PDT
Comment on attachment 258859 [details] [diff] [review]
Proposed fix for branch

dveditz, this one's ready to go (I can make the !NS_SUCCEEDED -> NS_FAILED change on checkin). The version mess was figured out and this will make the trunk and branch be in sync.
Comment 26 Johnny Stenback (:jst, jst@mozilla.com) 2007-03-23 13:35:37 PDT
I changed the !NS_SUCCEEDED() to NS_FAILED() on the trunk (non-functional change, so I just landed it).
Comment 27 Daniel Veditz [:dveditz] 2007-03-27 15:40:57 PDT
Comment on attachment 258859 [details] [diff] [review]
Proposed fix for branch

approved for 1.8.0.12/1.8.1.4, a=dveditz for release-drivers
Comment 28 Nickolay_Ponomarev 2007-04-02 06:41:16 PDT
timeless filed bug 376201 for outdated npapi.h documentation, but this feature probably should be documented too, right?
Comment 29 Johnny Stenback (:jst, jst@mozilla.com) 2007-04-04 17:25:19 PDT
Fixed on the branches.

For the record, a specification of sorts does exist for this (in the closed plugin-futures mailinglist), I'll see if I can extract that and get it up on the wiki...
Comment 30 Eric Shepherd [:sheppy] 2007-10-11 09:55:58 PDT
Did you ever get that "specification of sorts" extracted?  Would like to get this into docs.
Comment 31 Eric Shepherd [:sheppy] 2008-03-07 14:47:53 PST
Now documented at http://developer.mozilla.org/en/docs/NPStream

Note You need to log in before you can comment on or make changes to this bug.