870238 - Firefox dumps a warning on startup (Method userSpaceScaleFactor in class NSWindow is deprecated on 10.7 and later)

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[:Gijs (away/busy until Mar 10)]

The full warning is:

*** WARNING: Method userSpaceScaleFactor in class NSWindow is deprecated on 10.7 and later. It should not be used in new applications. Use convertRectToBacking: instead.

Comment 1

[Nickolay_Ponomarev]

The message comes from GetDPI implementation http://dxr.mozilla.org/mozilla-central/source/widget/cocoa/nsCocoaWindow.mm#2481
(See also discussion in bug 857126 about GetDPI.)

Apparently backingScaleFactor is a possible replacement for userSpaceScaleFactor
<https://developer.apple.com/library/mac/documentation/cocoa/reference/applicationkit/classes/NSWindow_Class/Reference/Reference.html>.

However the HiDPI patch for bug 674373 already added backingScaleFactor usage to GetDPI():

> CGFloat scaleFactor = [aWindow userSpaceScaleFactor];
> float dpi = (heightPx / scaleFactor) / (heightMM / MM_PER_INCH_FLOAT);
> return dpi * backingScale; // <<--- this

Seems like scaleFactor and backingScale cancel each other out when backingScale is available.

Comment 2

[:Gijs (away/busy until Mar 10)]

(In reply to Nickolay_Ponomarev from comment #1)
> The message comes from GetDPI implementation
> http://dxr.mozilla.org/mozilla-central/source/widget/cocoa/nsCocoaWindow.
> mm#2481
> (See also discussion in bug 857126 about GetDPI.)
> 
> Apparently backingScaleFactor is a possible replacement for
> userSpaceScaleFactor
> <https://developer.apple.com/library/mac/documentation/cocoa/reference/
> applicationkit/classes/NSWindow_Class/Reference/Reference.html>.
> 
> However the HiDPI patch for bug 674373 already added backingScaleFactor
> usage to GetDPI():
> 
> > CGFloat scaleFactor = [aWindow userSpaceScaleFactor];
> > float dpi = (heightPx / scaleFactor) / (heightMM / MM_PER_INCH_FLOAT);
> > return dpi * backingScale; // <<--- this
> 
> Seems like scaleFactor and backingScale cancel each other out when
> backingScale is available.

Thanks for the research! I would imagine that we could get the same info as userSpaceScaleFactor by taking the screen rect and converting it to a backingrect using convertRectToBacking, and dividing the heights? Not 100% sure though. Markus, does that seem plausible?

Comment 3

[Markus Stange [:mstange]]

Attachment: rip it out

On my HiDPI machine, userSpaceScaleFactor is always one. I think it's always been that way, except if you used QuartzDebug to override it on a pre-10.7 version of OS X.

Comment 4

[Jonathan Kew [:jfkthame]]

Comment on attachment 8421908 [details] [diff] [review]
rip it out

Review of attachment 8421908 [details] [diff] [review]:
-----------------------------------------------------------------

Yes - AIUI, this is basically a relic of an older Apple plan for resolution independence, but it never really got adopted. Let's get rid of it.

Comment 5

[Markus Stange [:mstange]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b2db2286258

Comment 6

[Ed Morley [:emorley]]

Push backed out for one of the commits inside it causing reftest + crashtest failures on OS X:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=4b2db2286258&jobname=reftest|crashtest

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/87349c65ee72
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/055fe4ccf919

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4b2db2286258

Comment 8

[:Gijs (away/busy until Mar 10)]

(In reply to Wes Kocher (:KWierso) from comment #7)
> https://hg.mozilla.org/mozilla-central/rev/4b2db2286258

In comment #6, this was listed as backed out. I'm now confused about whether this is in m-c or not. Did the push make it but not the backout? Are there just some checkins that aren't listed here? :-\

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/87349c65ee72

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to :Gijs Kruitbosch from comment #8)
> (In reply to Wes Kocher (:KWierso) from comment #7)
> > https://hg.mozilla.org/mozilla-central/rev/4b2db2286258
> 
> In comment #6, this was listed as backed out. I'm now confused about whether
> this is in m-c or not. Did the push make it but not the backout? Are there
> just some checkins that aren't listed here? :-\

Oops, grabbed the wrong changeset for my merge. Fixed.

Comment 11

[Markus Stange [:mstange]]

Relanded: https://hg.mozilla.org/integration/mozilla-inbound/rev/c1512bf25a70

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c1512bf25a70