Closed Bug 715024 Opened 8 years ago Closed 8 years ago

Get rid of nsCocoaFeatures

Categories

(Core :: Widget: Cocoa, defect)

x86
macOS
defect
Not set

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.
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)
Attachment #598638 - Attachment is obsolete: true
Attachment #598638 - 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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f577f905f3e
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
http://hg.mozilla.org/mozilla-central/rev/12f13acb5ea8
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.