847546 - Retina Display (HiDPI) not supported on SeaMonkey on MacOS X

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Cyrus Harmon]

Attachment: seamonkey-hidpi.patch

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_2) AppleWebKit/537.22 (KHTML, like Gecko) Chrome/25.0.1364.152 Safari/537.22

Steps to reproduce:

Retina Display (HiDPI) is supported on Firefox (?), but not SeaMonkey. Bug 785330 tracks overall support and there may be other issues, but enabling HiDPI mode seems to make things work OK. The attached path enables HiDPI mode.


Actual results:

Hi resolution mode should isn't enabled.


Expected results:

Hi resolution mode should should be enabled.

Comment 1

[Philip Chee]

Comment on attachment 720830 [details] [diff] [review]
seamonkey-hidpi.patch

Hi! Thanks for your patch. Without 2x graphics for our themes our toolbar buttons might appear rather blurry won't they? Perhaps we need a new icon set as well for OSX.

Comment 2

[[e-mail]]

Comment on attachment 720830 [details] [diff] [review]
seamonkey-hidpi.patch

Sorry, I have no idea what's going on here. I don't even know whether those are supposed to be spaces or tabs.

Comment 3

[Frank Wein [:mcsmurf]]

Confirming for now, looks like a bug.

Comment 4

[Stefan [:stefanh]]

Cyrus,

Thanks for the patch. I'll need to see if I can simulate HiDPI mode somehow since I don't have a retina mac. Perhaps you could attach a screenshot showing how our icons looks with/without HiDPI mode?

Interestingly, Firefox doesn't make use of NSHighResolutionCapable. Instead, they have a NSPrincipalClass entry.

Comment 5

[Philip Chee]

We probably also need something like Bug 849460 
https://hg.mozilla.org/mozilla-central/rev/3dac95162a78

Comment 6

[Stefan [:stefanh]]

I think we should use the NSPrincipalClass entry. We also need to fix a bunch of icons.

Comment 7

[Philip Chee]

> We also need to fix a bunch of icons.
I'm putting this down before I forget about this:

google.xml search icon:
http://hg.mozilla.org/mozilla-central/rev/f4beba4909b0#l6.12

image-rendering: -moz-crisp-edges;
http://hg.mozilla.org/mozilla-central/rev/f4beba4909b0#l7.11

Comment 8

[Ian Neal]

Comment on attachment 720830 [details] [diff] [review]
seamonkey-hidpi.patch

I agree with stefanh, we probably need to use NSPrincipalClass
As this needs changes and some icons/css f-

Comment 9

[Stefan [:stefanh]]

Comment on attachment 720830 [details] [diff] [review]
seamonkey-hidpi.patch

Per comment #6

Comment 10

[Ian Neal]

Should start by porting the two patches from Bug 764083

Comment 11

[Iacopo Benesperi [:iacchi]]

Just a quick update on this. Until a couple of versions ago (2.25?), adding

<key>NSHighResolutionCapable</key>
<true/>

at the end of info.plist worked pretty well, if not for the icons at least for the text. Starting with the release previous to 2.29 this doesn't work anymore. Any idea why and which workaround we can use now at least for the text?

Comment 12

[Stefan [:stefanh]]

You don't see any difference when using the NSPrincipalClass entry?

Comment 13

[Iacopo Benesperi [:iacchi]]

I'm not 100% sure about NSPrincipalClass, but having a look at Firefox's info.plist I've added this to the Seamonkey one:

<key>NSPrincipalClass</key>
<string>GeckoNSApplication</string>

but it changes nothing. Should have I done something different?

Comment 14

[Stefan [:stefanh]]

You should see a difference in text rendering (in the title bar, for example).

Comment 15

[Iacopo Benesperi [:iacchi]]

That's the point - I don't.

Comment 16

[Iacopo Benesperi [:iacchi]]

Don't ask me why, but updating from 10.9.2 to 10.9.4 (or maybe it's the rebooting that did the trick?) solved the issue. Now the text is ok.

Comment 17

[Stefan [:stefanh]]

I think the first step here is to add the key NSPrincipalClass, so if you can provide a patch for it, I'll be happy to review it :-)

Comment 18

[Serguei Trouchelle]

Attachment: Patch to allow correct rendering on retina display

Here's the patch that fixes font display on retina screens. There are some blurry icons still, but at least browser is usable.

Comment 19

[Stefan [:stefanh]]

Comment on attachment 8657334 [details] [diff] [review]
Patch to allow correct rendering on retina display

Thanks! This looks good, but in order to apply the patch we need a better format. Do you have mercurial installed? If you have and have a cloned comm-central repo, you can set up (in your ~/.hgrc file) the right format like this:

[diff]
git = 1
showfunc = 1
unified = 8
----------------
And then just do hg diff > mypatch.diff (or mypatch.patch) after you've changed the file. There are lots of mozilla guides on how to set up mercurial, but you should be fine for now with just the above settings.

In this case, a git style patch in unified format from the top source dir would also be fine.

You can ask me for review (just set the review flag to ? and put my email address in the requestee field) when you upload the new patch. I will r+ it and then I'll check in the patch for you.

Comment 20

[Matt Felix]

Attachment: HiDPI support patch

Hi! I tried to format the patch the way you suggested.

Comment 21

[Matt Felix]

Comment on attachment 8853698 [details] [diff] [review]
HiDPI support patch

Added review request

Comment 22

[Stefan [:stefanh]]

Comment on attachment 8853698 [details] [diff] [review]
HiDPI support patch

Thanks, this looks good!

-</dict>
+        <key>NSPrincipalClass</key>
+        <string>GeckoNSApplication</string>
+    </dict>
 </plist>

The indentation is a bit off here, but I can fix that on check-in.

Comment 23

[Stefan [:stefanh]]

https://hg.mozilla.org/comm-central/rev/1b7e88473e8331887940b597e0b8d18607c05a0d

Comment 24

[Frank-Rainer Grahl (:frg)]

Stefan the bug is missing in the comment. Could you back this out and push it with an updated comment.

Comment 25

[Stefan [:stefanh]]

Yeah, I noticed it and backed out the patch in https://hg.mozilla.org/comm-central/rev/900b3b2cc507 and re-landed in https://hg.mozilla.org/comm-central/rev/b40d588a73ba

Comment 26

[Stefan [:stefanh]]

Attachment: Patch with correct commit message

This should be low-risk and is supposed to increase support for Retina screens, so I think it's worth taking it on branches.