Closed
Bug 1338870
Opened 7 years ago
Closed 7 years ago
Review use of "super respondsToSelector" in nsCocoaWindow.mm
Categories
(Core :: Widget: Cocoa, defect)
Core
Widget: Cocoa
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: away, Assigned: mstange)
Details
Attachments
(1 file)
I don't speak Objective-C and merely stumbled across this code, so apologies if I'm missing something. Please see this code: https://dxr.mozilla.org/mozilla-central/rev/0eef1d5a39366059677c6d7944cfe8a97265a011/widget/cocoa/nsCocoaWindow.mm#3191,3211 My understanding of https://developer.apple.com/reference/objectivec/1418956-nsobject/1418583-respondstoselector is that "super respondsToSelector" will always be true in those cases in nsCocoaWindow.mm, since the subclass overrides those methods, so that test is probably not doing what was intended. Another thing that confuses me is that we're either calling on super or on NSWindow, but this code is in BaseWindow, so super _is_ NSWindow, is it not?
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(mstange)
Assignee | ||
Comment 1•7 years ago
|
||
Thanks for spotting this, I had no idea! (In reply to David Major [:dmajor] from comment #0) > Another thing that confuses me is that we're either calling on super or on > NSWindow, but this code is in BaseWindow, so super _is_ NSWindow, is it not? Well, super is an object whose class is NSWindow. The difference here is that [super frameRectForContentRect:aRect styleMask:aMask] calls the *instance* method on this object ("-[NSWindow frameRectForContentRect:styleMask:]"), and [NSWindow frameRectForContentRect:aRect styleMask:aMask] calls the *class* method ("+[NSWindow frameRectForContentRect:styleMask:]") which doesn't have access to this window object. The class variant of this method is documented / public, the instance method is undocumented and not guaranteed to be present.
Flags: needinfo?(mstange)
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8838639 [details] Bug 1338870 - Don't call [super respondsToSelector:] because it doesn't do what you might think. https://reviewboard.mozilla.org/r/113476/#review115014
Attachment #8838639 -
Flags: review?(spohl.mozilla.bugs) → review+
Pushed by mstange@themasta.com: https://hg.mozilla.org/integration/autoland/rev/805f47239008 Don't call [super respondsToSelector:] because it doesn't do what you might think. r=spohl
Updated•7 years ago
|
Assignee: nobody → mstange
Comment 5•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/805f47239008
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•