Closed
Bug 98641
Opened 23 years ago
Closed 13 years ago
Implement passing NPSavedData arg between NPP_Destroy() and NPP_New()
Categories
(Core Graveyard :: Plug-ins, defect, P2)
Core Graveyard
Plug-ins
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: serhunt, Assigned: peterl-bugs)
References
()
Details
(Keywords: testcase, topembed+, Whiteboard: [PL:BRANCH],edt_a3,edt_c3)
Attachments
(3 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
This bug is an off-spring of bug 85549, I filing this one as it is more specific
in what the real problem is and more general in what the effect of it could be.
Currently we just ignore the save_data parameter when we create a new plugin
instance always passing null. This causes e.g. Acrobat reader not to remember
the scroll position when we leave the page with a pdf document and come back.
Save_data is a general mean the plugin can use to restore anything it would like
to when user returns to the page with it.
*** Bug 85549 has been marked as a duplicate of this bug. ***
Sorry, forgot formal thing. Steps for reproduction and verification purposes:
1. load the above url, this is several page long pdf document
2. scroll it all the way down, there are some links at the bottom
3. click on one of them and leave the original document
4. click the browser Back button to return to the original document
Actual: the document displayed with the first page visible
Expected: the document should be displayed as we leave it -- last page in view.
4.x does it right.
Comment 3•23 years ago
|
||
Nominating nsbeta1 and mozilla1.0. This is one feature of the NPAPI still left
to be implemented but it's mentioned in the new SDK docs. Plus, Acrobat uses it.
Acrobat definitely needs this. It allows us to go back to the last
page a user was viewing in a particular PDF file. (This works currently
in Netscape 4.X and Mac IE. It does not work in Win IE, however.)
Comment 5•23 years ago
|
||
In NS4.x I think it was a big hack to get it to work that way. I implemented a
similar feature in NS4.x for java applets and it was indeed a big hack to make
the applets restore their previous state when going back or forward to them.
I'm not sure about the feasibility of doing the same in NS6.x. But I think it
can be achieved by plugins maintaining some sort of state information about the
child.
Session History does have an attribute to maintain scrollbar state.
nsILayoutHistoryState (member of nsISHEntry) maintains all layout related state
information including scrollbar position, form values etc .... for regular
pages. The layoutHistoryState maintained by session history probably doesn't
kick in when a plugin is displayed. It may be worth studying the possibility of
using this member for plugins too.
Comment 7•23 years ago
|
||
adding adt3 to status whiteboard as per discussion with beppe. this is not a
crasher, more like a nuisance to user.
Whiteboard: [ADT3]
I am implementing the mechanism to save saveddata pointers when we leave a page
with a plugin and will post a patch soon. Meanwhile, would appreciate ideas on
how to handle multiple plugins on a page. In general case we may have different
plugins or different instances of the same plugin and all of them may want to
use saveddata. Any thoughts how we can remember associations and hand right
pointer to right instance when we return to the page?
This patch lacks Mac build script changes, I need help with this.
Please try on Windows and Unix and review.
Reporter | ||
Comment 10•23 years ago
|
||
This patch is rather big, so I will add couple of words about what it does.
I added a new global object |nsPluginHistory| which keeps track of saved data
pointers. |nsPluginHistory| is created and deleted in |nsPluginHostImpl|
ctor/dtor and is passed to every instance of |ns4xPluginInstance| object where
it is stored as a member. There is no particular reason not to make it under the
ownership of the plugin host, but I chose not to add more mess to our huge and
hairy plugin host class. |nsPluginHistory| keeps a list of visited URLs (pages
with plugins), each element of this list is in turn a list of saved data
pointers as one URL may have multiple plugins or plugin instances. Both kinds of
lists are objects of |nsVoidArray| type. There is a limit on the number of
stored URL entries, but there is no limit on the number of saved data pointers
for each URL. I decided always to store |sdata|, whether plugin sets it or not,
otherwise I see no easy way to determine which plugin on the page wants its data
back and which does not.
Please comment and review.
Status: NEW → ASSIGNED
Keywords: patch
Updated•23 years ago
|
Priority: -- → P3
Target Milestone: --- → mozilla1.0
Comment 11•23 years ago
|
||
the spec on this issue confuses me, it has an example:
"a video player could save the last frame number to be displayed. When the user
returns to the page, the previous frame number is passed to the new instance of
the plug-in, so it can initially display the same frame."
what if page has 2 instances of the same player plugin and both want to save its
data?
Comment 12•23 years ago
|
||
What about using the browser's session history?
Reporter | ||
Comment 13•23 years ago
|
||
To peterl: nsISHistory is frozen, I did not look at it in depth but found no
obvious way to bind plugin data to it. Tracking plugin data is pretty trivial
task, so implementing it separately would not take much more time than finding a
proper way of hooking it to the session history. In addition, we have an
advantage of ownership. In case we need more complicated mechanism of
manipulating the history entries which the session history already has and find
a way to feed it with our data then I think it will be easy to hook up not even
changing |nsPluginHistory| API.
To serge: I don't quite understand what you are asking. If you mean what is
going to happen in such a case according to the old spec, then I do not know. If
your question is about the patch, then both data will be saved and restored when
needed. Here I assume that embedded objects are created and destroyed in the
same order. Which may be not the case, then we will have a bug and will need to
find a way to create associations.
Comment 14•23 years ago
|
||
>according to the old spec
is there new one?
>Here I assume that embedded objects are created and destroyed in the same order
And you also assumed the embedded objects are going to be the same for the given
url, is this correct?
>then we will have a bug and will need to find a way to create associations
why do not implement it now, to avoid future bugs?
Reporter | ||
Comment 15•23 years ago
|
||
>And you also assumed the embedded objects are going to be the same for the given
>url, is this correct?
No. I don't assume that. There are comments in the patch about this. It will
discard the data in such a case.
>>then we will have a bug and will need to find a way to create associations
>why do not implement it now, to avoid future bugs?
Any ideas? Besides, the assumption could be true.
Comment 16•23 years ago
|
||
Reporter | ||
Comment 17•23 years ago
|
||
Can we raise an ADT status of the bug? I think this is kind of important, and
given there already is a working patch, we probably don't want to loose the
functionality.
Comment 18•23 years ago
|
||
It has the appropriate setting, but I do not see an r=, sr= or drivers approval
Reporter | ||
Comment 19•23 years ago
|
||
What I mean, AFAIK today is the last day when ADT3 bugs will be allowed to go
in. This one still needs work.
Comment 20•23 years ago
|
||
Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt. If you have any
questions about this, please email adt@netscape.com. You can search for
"changing adt3 bugs" to quickly find and delete these bug mails.
Keywords: nsbeta1-
Comment 21•23 years ago
|
||
Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt. If you have any
questions about this, please email adt@netscape.com. You can search for
"changing adt3 bugs" to quickly find and delete these bug mails.
Keywords: nsbeta1+
Comment 23•22 years ago
|
||
The plug-ins triage team (av, beppe, peterl, serge and shrir) have reviewed this
issue and have made the following determination:
This is necessary and will be addressed within the next milestone, the patch
needs to be reviewed and modified as required.
Whiteboard: [ADT3] → [PL2:NA]
Updated•22 years ago
|
Target Milestone: mozilla1.1alpha → mozilla1.0.2
Updated•22 years ago
|
Comment 24•22 years ago
|
||
batch: adding topembed per Gecko2 document
http://rocknroll.mcom.com/users/marek/publish/Gecko/Gecko2Tasks.html
Keywords: topembed
Updated•22 years ago
|
Comment 25•22 years ago
|
||
moving to 1.4 beta, plug-in branch work
Whiteboard: [PL2:NA] → [PL:BRANCH]
Target Milestone: mozilla1.3alpha → mozilla1.4beta
Comment 26•22 years ago
|
||
jkesier gave me a strategy to use session history,
--->peterl
Assignee: av → peterl
Status: ASSIGNED → NEW
Updated•22 years ago
|
QA Contact: shrir → ashishbhatt
Updated•22 years ago
|
Whiteboard: [PL:BRANCH] → [PL:BRANCH],edt_a3,edt_c3
Updated•22 years ago
|
Target Milestone: mozilla1.4beta → mozilla1.5alpha
Comment 28•21 years ago
|
||
A further issue is that as the current code stands it doesn't
even delete the NPSavedData fully, so this leaks memory for
any plugin that tries to use this functionality.
The following should be done until this bug is fixed for real:
--- embedding/browser/activex/src/pluginhostctrl/nsPluginHostCtrl.cpp
@@ -725,6 +725,7 @@
if (pSavedData && pSavedData->buf)
{
NPN_MemFree(pSavedData->buf);
+ NPN_MemFree(pSavedData);
}
}
William Bardwell
wbardwel@curl.com
Comment 29•21 years ago
|
||
Please apply...and then do the hard work of fixing the bug for real...
Comment 30•18 years ago
|
||
It still leaks the NPSavedData object, although that code seems to have moved.
Updated•16 years ago
|
Hardware: PC → All
Target Milestone: mozilla1.5alpha → ---
Updated•15 years ago
|
QA Contact: ashshbhatt → plugins
Comment 31•13 years ago
|
||
I have never heard any mention of this feature in all my years of working on NPAPI specs and Mozilla's implementation . It isn't a commonly requested feature and it doesn't seem necessary. NPAPI plugins are trusted, they can store their own state in memory or on disk.
I'm open to arguments for doing this, but right now this seems like WONTFIX to me.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
Comment 32•13 years ago
|
||
(In reply to Josh Aas (Mozilla Corporation) from comment #31)
> I have never heard any mention of this feature in all my years of working on
> NPAPI specs and Mozilla's implementation . It isn't a commonly requested
> feature and it doesn't seem necessary. NPAPI plugins are trusted, they can
> store their own state in memory or on disk.
Could it possibly be because it doesn't work? If it did work, people would have use it. Nice API though:
https://developer.mozilla.org/en/NPSavedData
Shame it doesn't work as documented:
https://developer.mozilla.org/en/NPP_New
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•