Adding support for data uris in resolveUrl.

[nicolasnoble]

Fixes #5198.

[googlebot]

So there's good news and bad news.

:thumbsup: The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

:confused: The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

[TimvdLippe]

I had another stab at it and it is indeed failing. I will need to run this on Sauce later to debug :cry:

[nicolasnoble]

My guess is the onload trick isn't properly working everywhere for all browsers, but I am not sure how to solve it.

Worst case scenario, for readability, it might be relevant to just add a comment next to the link that shows what its contents are.

Also I would believe the test would be more relevant to use a Polymer component because that way it exercises the resolveURL codepath - your attempt doesn't.

I can rearrange my original PR to add the comment block if you want.

[TimvdLippe]

Yes lets resort to the comment. Strange thing is that it works just fine on Firefox and Chrome locally, but on CI it borks.

On Wed, 18 Apr 2018, 17:09 Nicolas Noble, [email protected] wrote:

My guess is the onload trick isn't properly working everywhere for all browsers, but I am not sure how to solve it.

Worst case scenario, for readability, it might be relevant to just add a comment next to the link that shows what its contents are.

Also I would believe the test would be more relevant to use a Polymer component because that way it exercises the resolveURL codepath - your attempt doesn't.

I can rearrange my original PR to add the comment block if you want.

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/Polymer/polymer/pull/5199#issuecomment-382421001, or mute the thread https://github.com/notifications/unsubscribe-auth/AFrDb4rep8dZXMGHbWrEs-cjGMEXkgl7ks5tp1cugaJpZM4TZEtT .

[googlebot]

CLAs look good, thanks!

[nicolasnoble]

Yep, I have the same experience: it works fine locally for me too, so I don't exactly know what's going on with the CI... Oh well :-)

I've updated the original PR to add a comment. PTAL.

[nicolasnoble]

Sure. Is there going to be a planned Polymer 2.0 release with this fix possibly in there however ? I wouldn't like to have to upgrade my project immediately to Polymer 3.0 just to get this fix in there.

[TimvdLippe]

Hm, I am actually not sure what happens in that case. I will discuss with the others and see what we can do!

[nicolasnoble]

Okay, thanks. I mean, most of the time, substantials projects still get "maintenance fixes and releases" for a little while before abandoning the previous version, and Polymer 2.6.0 got released less than a month ago. It'd be quite harsh to drop support for it, forcing people to migrate to get fixes like this one in there.

I mean, I could still monkey-patch my Polymer 2.6.0 during postinstall to incorporate this fix instead of waiting on a hypothetical 2.6.1, but I'd rather not.

[TimvdLippe]

Yeah I am definitely not saying we are dropping support, we simply havent decided yet what our plans are. We still support 1.x with bug fixes, so we will for certain do that with 2.x as well.

[nicolasnoble]

Cool, thanks! :)

[nicolasnoble]

Gentle ping. I would actually love to see this one backported to the older polymer 2.x branch too.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[nicolasnoble]

Yeah it's stale alright.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.