Closed
Bug 1100530
Opened 10 years ago
Closed 9 years ago
Clean up OS X version constants usage and definition
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: milan, Assigned: milan)
References
Details
Attachments
(1 file, 1 obsolete file)
13.42 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
See some of the discussion in bug 1094338. GfxInfo.mm and nsCocoaFeatures.mm define the same hex for OS X versions (e.g., 0x1060 for 10.6.0), we're using "0x1000" instead "A" for version 10, we don't have DRIVER_OS_OS_X_10_9 or DRIVER_OS_OS_X10_10 defined, etc.
Assignee | ||
Comment 1•10 years ago
|
||
Having said that, I kind of like 0x1000 to be version 10 and 0x1100 to be version 11, but it does mess up for the minor version, which is at "A" already (0x10A0 is 10.10.0).
Assignee | ||
Comment 2•10 years ago
|
||
OK, looking at where we are right now, we should stick with 0x1000 as the base for version 10. There are way too many places that assume it.
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8524850 -
Flags: feedback?(jgilbert)
Comment 4•10 years ago
|
||
Comment on attachment 8524850 [details] [diff] [review] Clean up the code to only have constants in one place Review of attachment 8524850 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/xre/nsNativeAppSupportCocoa.mm @@ +88,5 @@ > // which will make the browser quit. In principle we could display an > // alert here. But the alert's message and buttons would require custom > // localization. So (for now at least) we just log an English message > // to the console before quitting. > + if (major < 10 || minor < 6) { IsAtLeastVersion?
Attachment #8524850 -
Flags: feedback?(jgilbert) → feedback+
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Jeff Gilbert [:jgilbert] from comment #4) > Comment on attachment 8524850 [details] [diff] [review] > Clean up the code to only have constants in one place > > Review of attachment 8524850 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: toolkit/xre/nsNativeAppSupportCocoa.mm > @@ +88,5 @@ > > // which will make the browser quit. In principle we could display an > > // alert here. But the alert's message and buttons would require custom > > // localization. So (for now at least) we just log an English message > > // to the console before quitting. > > + if (major < 10 || minor < 6) { > > IsAtLeastVersion? Sadly, IsAtLeastVersion(), together with most other methods for version checking assumes that anything less than major 10 should be set to 10, and for major 10, anything less than minor 6 should be set to 6. So, it will always return true to IsAtLeastVersion(10, 6).
Assignee | ||
Comment 6•10 years ago
|
||
Mostly rebasing
Attachment #8524850 -
Attachment is obsolete: true
Attachment #8530440 -
Flags: review?(mstange)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → milan
Updated•10 years ago
|
Attachment #8530440 -
Flags: review?(mstange) → review+
Assignee | ||
Comment 7•9 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=7cc6c51a02ac
Keywords: checkin-needed
Comment 8•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f25e20a0f238
Keywords: checkin-needed
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f25e20a0f238
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in
before you can comment on or make changes to this bug.
Description
•