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

Enhancement: mat-icon set current page absolute path on filter urls for compatibility with safari #9276

Closed
MikaAK opened this issue Jan 8, 2018 · 9 comments · Fixed by #12428
Assignees
Labels
feature This issue represents a new feature or feature request rather than a bug or bug fix needs: discussion Further discussion with the team is needed before proceeding P4 A relatively minor issue that is not relevant to core functions

Comments

@MikaAK
Copy link

MikaAK commented Jan 8, 2018

Feature request:

mat-icon is super useful for using svg's inline in the browser. However what's the solution when you need to filter. Since <base href='/'> is in use for history you must append svg filters with the current url.
Example:

<svg viewBox="0 0 96 96" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
  <defs>
    <mask id="b" fill="#fff">
      <use xlink:href="#a"/>
    </mask>
  </defs>
  <path d='SOMEAMOUNTOFPOINTS' mask='url(http://example.com/my-page#b)'
</svg>

This works because url is set to absolute path. Anything else and the svg works in chrome but not in safari! This used to be an issue in chrome a few years ago but it seems chrome got smarter, however safari has not 😢

What are the steps to reproduce?

I tried to reproduce on plunker but since there's no baseUrl being set and it doesn't seem possible to set I cannot reproduce.

What is the use-case or motivation for changing an existing behavior?

Safari Compatibility

Other Info

As a workaround you can skip setting baseUrl in the index document and set it with

import {APP_BASE_HREF} from '@angular/common'

providers: [{provide: APP_BASE_HREF, useValue: '/'}]
@jelbourn jelbourn added feature This issue represents a new feature or feature request rather than a bug or bug fix discussion P4 A relatively minor issue that is not relevant to core functions labels Jan 9, 2018
@jelbourn
Copy link
Member

jelbourn commented Jan 9, 2018

Possible duplicate of #4263

@MikaAK
Copy link
Author

MikaAK commented Jan 10, 2018

Not quite @jelbourn that issue seems to be around allowing urls passed to the register to be relative. This is more around either changing the actual url's of the svg's filters/masks when referencing defs (since it must change on a per page basis this would need to be done at render).

see https://github.com/jeffbcross/angular-svg-base

@MikaAK
Copy link
Author

MikaAK commented Jan 17, 2018

New findings, the workaround I provided above doesn't seem to work on subroutes. For example when navigating to http://localhost:3000/pain-module/quieting-the-pain-alarm it tries to request packages from http://localhost:3000/pain-module/

With this new addition, this makes svg filters unusable in safari

@tenbits
Copy link

tenbits commented Jun 27, 2018

Also clip-path="url(#a)" doesn't work in safari. Painful.

MikaAK added a commit to MikaAK/material2 that referenced this issue Jun 27, 2018
SVG filters do not work in Safari/FF because of paths in url needing to be updated to current path
This fixes angular#9276
@crisbeto crisbeto self-assigned this Jul 29, 2018
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 29, 2018
Prepends the current path to any SVG elements with attributes pointing to something by id. If the reference isn't prefixed, it won't work on Safari if the page has a `base` tag (which is used by most Angular apps that are using the router).

Fixes angular#9276.
jelbourn pushed a commit that referenced this issue Aug 23, 2018
Prepends the current path to any SVG elements with attributes pointing to something by id. If the reference isn't prefixed, it won't work on Safari if the page has a `base` tag (which is used by most Angular apps that are using the router).

Fixes #9276.
jelbourn pushed a commit that referenced this issue Aug 25, 2018
Prepends the current path to any SVG elements with attributes pointing to something by id. If the reference isn't prefixed, it won't work on Safari if the page has a `base` tag (which is used by most Angular apps that are using the router).

Fixes #9276.
crisbeto added a commit to crisbeto/material2 that referenced this issue Sep 1, 2018
Prepends the current path to any SVG elements with attributes pointing to something by id. If the reference isn't prefixed, it won't work on Safari if the page has a `base` tag (which is used by most Angular apps that are using the router).

Fixes angular#9276.
crisbeto added a commit to crisbeto/material2 that referenced this issue Oct 5, 2018
Prepends the current path to any SVG elements with attributes pointing to something by id. If the reference isn't prefixed, it won't work on Safari if the page has a `base` tag (which is used by most Angular apps that are using the router).

Fixes angular#9276.
vivian-hu-zz pushed a commit that referenced this issue Oct 5, 2018
Prepends the current path to any SVG elements with attributes pointing to something by id. If the reference isn't prefixed, it won't work on Safari if the page has a `base` tag (which is used by most Angular apps that are using the router).

Fixes #9276.
@FingalP
Copy link

FingalP commented Oct 17, 2018

We tried upgrading to Material 7, and this broke icons on our header in Chrome. As they are in our header, they get rendered on the first page which is opened, and the url is set to that path. Once we navigate from there, they aren't re-rendered, so the url is now wrong, and they vanish.

@MikaAK
Copy link
Author

MikaAK commented Oct 17, 2018

That is a probably issue with doing this and having icons persistant across pages. The icons would need a url update

@FingalP
Copy link

FingalP commented Oct 18, 2018

It turns out, all of our icons are broken, not just the ones which are persistent across pages, as the _location is cached by the location factory.
However, a fix should also also consider supporting icons which are persistent across pages.

@crisbeto
Copy link
Member

#13641 should sort the issue out. My understanding is that the URLs don't have to be updated once they're in place, but the problem is that the provider caches the path the first time it's resolved.

roboshoes pushed a commit to roboshoes/material2 that referenced this issue Oct 23, 2018
Prepends the current path to any SVG elements with attributes pointing to something by id. If the reference isn't prefixed, it won't work on Safari if the page has a `base` tag (which is used by most Angular apps that are using the router).

Fixes angular#9276.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
@mmalerba mmalerba added the needs: discussion Further discussion with the team is needed before proceeding label Mar 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature This issue represents a new feature or feature request rather than a bug or bug fix needs: discussion Further discussion with the team is needed before proceeding P4 A relatively minor issue that is not relevant to core functions
Projects
None yet
6 participants