Closed Bug 587345 Opened 10 years ago Closed 10 years ago

deAgnostify nsXULPDGlobalObject some

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: Ms2ger, Assigned: Ms2ger)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

Attached patch Patch v1 (obsolete) — Splinter Review
Like bug #575431.
Attachment #466010 - Flags: review?(jst)
Comment on attachment 466010 [details] [diff] [review]
Patch v1

While changing nsnull being returned as an nsresult to NS_OK is a good chang :) I don't think we should be changing nsnull to NULL when actually dealing with pointers in C++ code.

r=jst either way.
Attachment #466010 - Flags: review?(jst) → review+
Comment on attachment 466010 [details] [diff] [review]
Patch v1

I understood we were moving toward more standard C++, including NULLs, but that might just be bsmedberg.
Attachment #466010 - Flags: approval2.0?
What's the win here that would induce us to take this now, instead of after branching?
Comment on attachment 466010 [details] [diff] [review]
Patch v1

I think we should wait until we've branched with this.
Attachment #466010 - Flags: approval2.0? → approval2.0-
Sure.
Whiteboard: [needs landing]
Status: NEW → ASSIGNED
Depends on: post2.0
Blocks: 620000
I tried to land this on cedar, but it doesn't apply cleanly.  Please submit a new version of the patch (or land it on cedar yourself).
Attached patch Patch for checkin (obsolete) — Splinter Review
This one should apply. Thanks!
Attachment #466010 - Attachment is obsolete: true
http://hg.mozilla.org/projects/cedar/rev/05369f93c49a
Whiteboard: [needs landing] → [needs landing][fixed-in-cider]
Whiteboard: [needs landing][fixed-in-cider] → [needs landing][fixed-in-cedar]
Backed out because it broke the build:

http://hg.mozilla.org/projects/cedar/rev/4f0c8bd1a8fa
Whiteboard: [needs landing][fixed-in-cedar] → [needs landing][not-ready]
Attached patch Patch for checkin (obsolete) — Splinter Review
That's why we should not have assertions.
Attachment #520961 - Attachment is obsolete: true
Third time's the charm, right?
Attachment #520985 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/83b87584d902
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing][not-ready]
Target Milestone: --- → mozilla2.2
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.