Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safari mask icon resolution / viewBox #242

Closed
phbernard opened this issue May 29, 2016 · 10 comments
Closed

Safari mask icon resolution / viewBox #242

phbernard opened this issue May 29, 2016 · 10 comments
Labels
bug
Milestone

Comments

@phbernard
Copy link
Contributor

@phbernard phbernard commented May 29, 2016

Currently, the Safari mask icons created by RFG have a viewBox of 700x700:

<svg version="1.0" xmlns="http://www.w3.org/2000/svg" width="700.000000pt" height="700.000000pt" viewBox="0 0 700.000000 700.000000" preserveAspectRatio="xMidYMid meet">

However, it should be 16x16.

Although RFG icons do work, it would be better to strictly follow Apple's recommendations.

@ghost
Copy link

@ghost ghost commented Jul 12, 2016

Same here. SVG icon does not display well.

@phbernard
Copy link
Contributor Author

@phbernard phbernard commented Jul 13, 2016

Really? Although the viewBox is not correct, all my experiments were successful and the icon is always displayed correctly by Safari.

Are you sure the mask icon is properly declared in your site? (you can run the checker for a quick review) Could you attach the SVG to this issue? (or please mail it to me if that's not possible).

@ghost
Copy link

@ghost ghost commented Jul 13, 2016

It does not display neither in Safari, Safari Technology Preview nor Firefox Developper Edition.

Checker does not work as check never ends.

I've send the SVG file by mail.

@MoOx
Copy link

@MoOx MoOx commented Sep 8, 2016

In latest macOS Sierra version, the produced svg file do not work on Safari when tab is pinned. I have just nothing.

I have double checked every parameters.

screen shot 2016-09-08 at 16 42 37

@phbernard
Copy link
Contributor Author

@phbernard phbernard commented Sep 8, 2016

Damn! Thank you for reporting.

So I wanted to reproduce the issue, only to discover that I have to pay 99€ to download a beta product I'll have for free when it's ready. Procrastination suddenly became my best ally.

What you observe is unexpected because Safari usually doesn't behaves like this when there is a problem with the mask icon (eg. when the color attribute is absent). It usually falls back to its default behavior, which is to generate an icon with the first letter of the domain name (so pinning example.com creates a "E" icon). Two hypothesis:

  1. Safari's behavior changes with Sierra. As simple as this.
  2. Safari doesn't scale the icon anymore. If this hypothesis is true, then what you see is the 16x16 top left corner of your much larger icon (which would be empty in this example).

I updated the compatibility test, and somehow hacked it to compare the "classic", buggy, high resolution icon and the expected, 16x16 icon. Could you run it?

  • The first test has a 16x16 icon. You should see the expected result, which is:

image

If you don't see this, it means that the resolution of the icon is not the culprit.

  • The second test still has a high resolution icon, so this time the icon should be wrong. If hypothesis 2 is true, you should see an isosceles right triangle.
@phbernard phbernard added this to the Package v0.14 milestone Sep 9, 2016
@phbernard
Copy link
Contributor Author

@phbernard phbernard commented Sep 15, 2016

I finally installed macos Sierra (which is available for free after all).

I don't reproduce the issue: the icon is displayed correctly, whatever the viewBox (16x16 or a lot more).

@MoOx Could you paste your master image and the parameter you used to generate the SVG icon for Safari? Or, as an alternative, could you send me your SVG by email?

@MoOx
Copy link

@MoOx MoOx commented Oct 4, 2016

I can't find it again. Will reopen if I get this again.

@MoOx
Copy link

@MoOx MoOx commented Oct 4, 2016

Oh I can't close this :/

@phbernard
Copy link
Contributor Author

@phbernard phbernard commented Nov 3, 2016

Fixed in branch package_v0_14

@phbernard
Copy link
Contributor Author

@phbernard phbernard commented Jan 18, 2017

Deployed a minute ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.