Closed
Bug 715024
Opened 12 years ago
Closed 12 years ago
Get rid of nsCocoaFeatures
Categories
(Core :: Widget: Cocoa, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: zpao, Assigned: heycam)
Details
Attachments
(1 file, 2 obsolete files)
Everything in it is in nsToolkit (with minor differences). It looks like it's only used by WebGLContextGL.cpp but I can't tell if it's actually needed there. It looks like we can just us nsToolkit instead.
Assignee | ||
Comment 1•12 years ago
|
||
We can't include nsToolkit.h in the files that currently use nsCocoaFeatures since they're not Objective-C files. What we could do is have nsToolkit derive from nsCocoaFeatures and eliminate the duplicate functions.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #598638 -
Flags: review?(bgirard)
Assignee | ||
Comment 3•12 years ago
|
||
(This might be bug 676907, too.)
Comment 4•12 years ago
|
||
Sorry it took me so long to respond. I'm not a fan of having nsToolkit extend nsCocoaFeatures. Wouldn't it be simpler to refactor the code to use nsCocoaFeatures instead?
Assignee | ||
Updated•12 years ago
|
Attachment #598638 -
Attachment is obsolete: true
Attachment #598638 -
Flags: review?(bgirard)
Comment 6•12 years ago
|
||
Comment on attachment 603917 [details] [diff] [review] Remove duplicate functions on nsCocoaFeatures and nsToolkit. (v1.1) We don't need to expose SupportCoreAnimationPlugins in nsToolkit, so r+ with that changes. It would of been nice to just remove the function from nsToolkit and replace any usage of nsToolkit OSVersion to use nsCocoaFeatures but this is already an improve so i'd be happy to take it as-is. If you're not aware you can use mxr.mozilla.org to where find the functions are used. Also feel free to ping and/or reassign review if it takes too long in the future.
Attachment #603917 -
Flags: review?(bgirard) → review+
Assignee | ||
Comment 7•12 years ago
|
||
You're right, replacing the nsToolkit::Blah() calls to nsCocoaFeatures::Blah() would be better. Re review time tbh I just requested review and forgot about the bug since it wasn't anything urgent. :)
Assignee | ||
Updated•12 years ago
|
Attachment #603917 -
Attachment is obsolete: true
Reporter | ||
Comment 8•12 years ago
|
||
Cameron, is this fine to land? (no rush, I was looking at these files again and got reminded about this)
Assignee | ||
Comment 9•12 years ago
|
||
Yes, I just forgot about this bug. Will land momentarily.
Assignee | ||
Comment 10•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f577f905f3e
Target Milestone: --- → mozilla15
Version: unspecified → Trunk
Assignee | ||
Comment 11•12 years ago
|
||
Backed out because I was a cowboy and didn't re-run my two month old patch through try: https://hg.mozilla.org/integration/mozilla-inbound/rev/521cd92d3c08
Assignee | ||
Comment 12•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/12f13acb5ea8
Comment 13•12 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/12f13acb5ea8
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•