Closed Bug 80083 Opened 24 years ago Closed 23 years ago

[xpcdom] window.navigator not rooted

Categories

(Core :: DOM: Core & HTML, defect, P1)

x86
Windows 2000
defect

Tracking

()

RESOLVED FIXED
mozilla1.0

People

(Reporter: bc, Assigned: jst)

Details

(Keywords: dataloss, testcase, Whiteboard: [ADT2 RTM][FIXED ON TRUNK])

Attachments

(2 files)

window.navigator is not rooted. Any properties added to window.navigator by a JS script, will be lost when Garbage Collection runs. Mozilla Trunk build 2001-05-09-15/win2k Steps to reproduce: 1. assign a property to window.navigator, i.e, window.navigator.testProperty = 'foo'; 2. iterate over a loop while creating/destroying objects and testing the value of window.navigator.testProperty Expected results: window.navigator.testProperty remains defined. Actual results: After 4096 iteratons, window.navigator.testProperty becomes undefined.
Attached file testcase
adding testcase keyword
Keywords: testcase
Priority: -- → P1
Target Milestone: --- → mozilla0.9.3
Possible nsBranch once we have a fix... work on it, man! ;)
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Moving to mozilla0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Target Milestone: mozilla0.9.5 → mozilla0.9.6
Pushing to mozilla0.9.7
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Keywords: nsbeta1
Pushing to mozilla1.0
Target Milestone: mozilla0.9.9 → mozilla1.0
Adding ADT need info. Does this affect any top sites?
Whiteboard: [ADT need info]
Attached patch Proposed fix.Splinter Review
Comment on attachment 80077 [details] [diff] [review] Proposed fix. sr=vidur. I really don't like the fact that we have to explictly "root" window properties (and DOM properties in general) this way. I don't doubt that we're going to hit cases in the future where developers rely on the persistence during the lifetime of a page of properties they set on DOM objects. A more generic solution that keeps XPConnect wrappers around would be more ideal.
Attachment #80077 - Flags: superreview+
Comment on attachment 80077 [details] [diff] [review] Proposed fix. r=fabian for what's it worth, I read the following on www.dithered.com: Last week, I re-wrote the Browser Detect Lite script. Instead of creating a new object to hold all the properties created by the script, I decided to use the navigator object. Logically, that's where those properties would belong. Unfortunately, Mozilla doesn't like expando properties in the navigator object (or any other top-level object like history, location, Math, or screen ). The properties hang around for a couple seconds after load and then get removed and become undefined. Properties can be created safely in window (this is how top-level variables and functions are created) and document . I can't find any documentation that says why expando properties aren't allowed on all browser-created objects, whether this is a bug, or how to fix it. ----------------------- Spot on :-)
Attachment #80077 - Flags: review+
jst checked this in some time ago. Marking FIXED.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Nonono, it's fixed on the trunk, but we should get this in on the branch as well. Reopening until it's fixed on the branch too.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [ADT need info] → [ADT need info][FIXED ON TRUNK]
Whiteboard: [ADT need info][FIXED ON TRUNK] → [ADT2 RTM][FIXED ON TRUNK]
Comment on attachment 80077 [details] [diff] [review] Proposed fix. a=chofmann,brendan please check in to 1.0 branch asap to get this in rc3.
Attachment #80077 - Flags: approval+
adding adt1.0.0+. Please checkin to the 1.0 branch.
Keywords: adt1.0.0adt1.0.0+
Fixed on the branch too.
Status: REOPENED → RESOLVED
Closed: 23 years ago23 years ago
Keywords: fixed1.0.0
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: