Closed Bug 1100530 Opened 5 years ago Closed 5 years ago

Clean up OS X version constants usage and definition

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: milan, Assigned: milan)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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).
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.
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+
(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: nobody → milan
Attachment #8530440 - Flags: review?(mstange) → review+
https://hg.mozilla.org/mozilla-central/rev/f25e20a0f238
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.