Closed Bug 715024 Opened 8 years ago Closed 8 years ago
Get rid of ns
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.
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
(This might be bug 676907, too.)
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?
OK, that works too.
Attachment #603917 - Flags: review?(bgirard)
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+
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. :)
Attachment #603917 - Attachment is obsolete: true
Cameron, is this fine to land? (no rush, I was looking at these files again and got reminded about this)
Yes, I just forgot about this bug. Will land momentarily.
Target Milestone: --- → mozilla15
Version: unspecified → Trunk
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
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.