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

<!--! do not remove --> inserted by polymer build / polymer serve causes warning in IE11, and result in blank polymer-2-starter-kit #2465

Open
LawrenceMok opened this issue Jun 1, 2017 · 54 comments

Comments

@LawrenceMok
Copy link

what I did:
polymer init
select polymer-2-starter-kit
polymer serve
open the site in IE11, got a completely blank page, console shows these warnings:

HTML1414: Unexpected character: U+0021 EXCLAMATION MARK (!)
127.0.0.1:8081 (67,9)
HTML1416: Unexpected character in comment end. Expected "-->".
127.0.0.1:8081 (67,10)

Also after around 10 seconds, got this error:
SCRIPT445: Object doesn't support this action
CustomElementInternals.js (255,7)

view source showing line 67 as:
<!--! do not remove -->

which should be inserted by polymer build / polymer serve

@colincannon-acp
Copy link

colincannon-acp commented Jun 1, 2017

Please help with this, as far as I can tell Polymer 2 does not work at all with ie11 after being built.

I removed the line

<!--! do not remove -->

After building and the original error was gone, but a different error popped up complaining about an unexpected end of file, and the page still did not display in the browser.

screen shot 2017-06-01 at 9 53 47 am

Also, I am getting the same error messages and problems in EDGE.

@stramel
Copy link
Contributor

stramel commented Jun 1, 2017

I was able to reproduce this as well. Although, I believe this issue would be better off being in the PSK repository.

Versions & Environment

  • Polymer CLI: 1.1.0
  • node: 6.9.1
  • Operating System: Windows 10

Steps to Reproduce

  1. Install polymer-cli (`npm i -g polymer-cli)
  2. Create a directory for the app and change to that directory (mkdir test && cd test)
  3. Initialize polymer-2-starter-kit (polymer init polymer-2-starter-kit)
  4. Serve app (polymer serve -o)

Expected Results

App loads successfully in all supported browsers

Actual Results

App fails to load in IE11. Also, throws warnings in IE and Edge.

Browsers Affected

  • Chrome
  • Firefox
  • Safari 10 (Untested)
  • Safari 9 (Untested)
  • Edge (Warnings)
  • IE 11 (Fails to Load and Warnings)

Console Output

Edge:
image

IE11:
image

@FredKSchott
Copy link
Contributor

Hmm, okay we'll need to make time to dig in to why this is happening.

@usergenic
Copy link
Contributor

Just updated bundler to preserve <!--! comments incidentally in Polymer/polymer-bundler#546

@ghost
Copy link

ghost commented Jun 23, 2017

Not even a quick workaround, just for the time being???

@cgriffin4
Copy link

@andreabioli - I was able to get my page to work after a few hours of fiddling with things but unfortunately I don't remember exactly what I did. Sorry, and good luck.

I know I tried straight up editing the index file and couldn't get things working, tried using different versions of webcomponentsjs and didn't have any success. Then I uninstalled polymer-cli completely. Installed the newest version of polymer-bundler and then reinstalled polymer-cli. I think I also made some changes to the index file and my-app file but I'm not sure which of those changes were related to getting things working again.

As a side note, this kind of timeline on bugs which break Polymer-Cli on windows or IE/Edge really has me hesitant to ever upgrade after finding a working version and worried about the future use of polymer at my workplace.

@ghost
Copy link

ghost commented Jun 23, 2017

...your last sentence is really scaring, but I think it's honestly objective.
I got to love Polymer approach, clean and modern: we are developing a test project here where I work now, but very often I get puzzled by these things...
It will be really a nightmare, if a different version of just one single component can break everything so easily... :-(

@justinfagnani
Copy link
Contributor

I tried reproducing just the comment warning with the following jsbin in IE11, but couldn't: https://jsbin.com/kiditugizo/edit?html,output

And from reading this page: https://msdn.microsoft.com/en-us/library/hh180764(v=vs.85).aspx I don't think that warning should break the page. It might be two different things.

@stramel
Copy link
Contributor

stramel commented Jun 23, 2017

@justinfagnani I was noticing it today still on PSK#2.0-preview.

@cgriffin4
Copy link

It will be really a nightmare, if a different version of just one single component can break everything so easily...

@andreabioli - It's not all that doom and gloom. For the most part versions between components work extremely well. The only issue I've seen has been updates/releases to new dev/build tools tend to have more bugs (sometimes crippling ones) in Windows OS/browsers. Polymer library and polymer components have been solid for the most part. That said, the tools are improving all the time and it's just generally not a good idea to immediately update production build tools so just take best practices and you'll be good.

For this issue it was certainly more than just the comment but it was how the build tool output the index file, specifically the section with the es5-custom-adapter.

That said, with the latest version of PSK (2.0-preview), version 1.2 of Polymer-Cli, and 1.5.1 of Polymer-Build I DO NOT see the problem any longer. @stramel - Could you provide versions or code where it is still an issue?

@ghost
Copy link

ghost commented Jun 23, 2017

You are right, sorry for having a moment of bad mood, but in the afternoon I had another big problem just trying to use iron-validator-behavior (still in 'old mode') in Polymer 2: I filed the problem, and I already saw I'm not the only one... These things don't help to consider the whole approach in a very positive way, but I'm confident that you can progress through all of this! :-)
Moreover, it's never a good think to criticize the (strong) efforts of other people, so sorry also for this!

@rpapeters
Copy link

rpapeters commented Jun 23, 2017 via email

@cgriffin4
Copy link

@rpapeters I'd check your version of polymer-build and download a fresh psk preview 2. Hopefully you'll notice one of those has been updated and it'll narrow down where your issue is. Good luck

@ghost
Copy link

ghost commented Jun 27, 2017

I made further tests.
I see for example that only in the case of IE11 and Edge there is an incredible high CPU activity before serving the page (is it normal in IE and Edge, or is it only for Polymer applications? I don't know...), and then, beyond the warnings, that as someone else said, maybe don't have an impact on displaying the page, these are the errors I get:

SCRIPT438: Object doesn't support property or method 'call'
patch-events.js (353,7)
SCRIPT445: Object doesn't support this action
CustomElementInternals.js (255,7)

That are probably the real responsible for NOT showing the page at all, in the end...
To note that the second error always happens, the first only sometimes... just to add more strangeness...

@bkawk
Copy link

bkawk commented Jun 27, 2017

I too have the same issue you can see it at https://www.bkawk.com

screen shot 2017-06-27 at 8 01 11 am

screen shot 2017-06-27 at 8 02 51 am

@ghost
Copy link

ghost commented Jun 27, 2017

I can add even more: The class defined in CustomElementInternals.js is CustomElementRegistry, that at this page, https://developer.mozilla.org/en-US/docs/Web/API/CustomElementRegistry, is said to be NOT supported in Internet Explorer.
I'm starting to suspect something... :-(
Tracing the code, the problem is (probably) that in that class, the function _flush() is called with an internal variable (this._internals, line 142 of the file I have) that is undefined, and it's trying to call this._internals.patchAndUpgradeTree(document), that obviously can only fail...

@ghost
Copy link

ghost commented Jun 27, 2017

Now I tried to jump over the offending line (CustomElementRegistry.js, line 142, this._internals.patchAndUpgradeTree(document);), and now I have no errors, only the warnings, but the page is totally blank, it looks like Polymer is not initialized at all...

@ghost
Copy link

ghost commented Jun 27, 2017

...and if we run the validator on bkawk page:
https://validator.w3.org/nu/?doc=https%3A%2F%2Fwww.bkawk.com
We see a couple of minor errors, and then, this time NOT marked as a warning, an error regarding the famous/infamous line with the comment...

@ghost
Copy link

ghost commented Jul 3, 2017

What worries me is this: nobody in the world can deliver at the moment a Polymer 2 application that works on IE???
Is there any update on this' High priority' bug? I'm really worried, now, because we have to go in production before the end of the month, and the reference browser is just IE! :-(
Sorry for asking...

@rpapeters
Copy link

rpapeters commented Jul 3, 2017 via email

@ghost
Copy link

ghost commented Jul 3, 2017

For me it's just a no point: I pushed to use Polymer, in this application, that will be run all over the world. Big numbers, in this, I cannot ask such a big company to make such a move (change browser!) just because of my (small) application... :-(((

@HughxDev
Copy link

HughxDev commented Jul 3, 2017

I agree with @cgriffin4 and @andreabioli. This issue is blocking an app launch for my client, which is costing me time/money/reputation. And yeah, I saved IE testing till the end, which was a misstep, but Polymer says it supports IE11+, so I was overly confident that any IE-specific issues would be minor; not something like the entire app failing to load. I can’t even get things to work by side-stepping the cli with a custom gulp build that basically just does crisper and babel. It looks like I’m going to have to spend time downgrading the app to Polymer 1.9, since a previous 1.x project loads fine in IE. This honestly makes me reconsider using the library in production environments. I would expect this kind of error from a beta, but not a 2.0 release.

@ghost
Copy link

ghost commented Jul 3, 2017

Just the same steps I did, @hguiney! :-) I'm still telling the customer 'don't worry, test it in Chrome, and when we go in production we will have the compatibility', but now I must prepare to downgrade to Polymer 1.x... :-(
Really not a good thing... :-(

@ghost
Copy link

ghost commented Jul 4, 2017

I had some hopes when I saw there was a new version of Polymer CLI.
Unfortunately, I could quickly realize that even with Polymer CLI 1.3.0 this problem is still totally there... :-(

@ghost
Copy link

ghost commented Jul 5, 2017

I FOUND THE PROBLEM!!!
After a lot of research, I saw that the 'Shop' Polymer 2 Application is working in IE, so I starting 'disassembling' that application, inserting my own code (and the needed code from what was generated by the starter kit (or CLI, whatever... you know...)
In the end it came out that in the my-app custom element the app-route is defined as follows:

    <app-route
        route="{{route}}"
        pattern="[[rootPattern]]:page"
        data="{{routeData}}"
        tail="{{subroute}}"></app-route>

Where the rootPattern variable is initialized like this:

      constructor() {
        super();

        // Get root pattern for app-route, for more info about `rootPath` see:
        // https://www.polymer-project.org/2.0/docs/upgrade#urls-in-templates
        this.rootPattern = (new URL(this.rootPath)).pathname;
      }

But the URL object is not compatible with IE (11), as we can see from this page:

https://caniuse.com/#search=url

And it blows up everything, in a very strange way, that's not easy to spot.
So now I changed the route as in:

<app-route route="{{route}}" pattern="/:page" data="{{routeData}}" tail="{{subroute}}"></app-route>

Without the variable at all, of course removing the offending line! And it works without any problem, in any browser...

Do I deserve a prize, for one full day of tests? :-)

P.S.: now I'm curious... Please, could the other users check to see if this solve the problem?

@ghost
Copy link

ghost commented Jul 5, 2017

And in this official page:
https://developer.mozilla.org/en-US/docs/Web/API/URL/URL
They explicitly say that 'This is an experimental technology', and even in this page it's stated that this is NOT supported in IE11.
Using this class in a generic and global code generator is really a little bit crazy! :-))))))))))))))

@krumware
Copy link
Contributor

krumware commented Jul 5, 2017

related issue: Polymer/polymer-starter-kit#1020

I'm a bystander in this one, but I'm guessing the version of the PSK which is pulled by the current polymer-cli is not the most up-to-date

@jsilvermist
Copy link
Contributor

jsilvermist commented Jul 5, 2017

@andreabioli This is already fixed in the latest PSK branches, and will probably be released as soon as lazy-imports are finalized and implemented in the PSK.

A URL polyfill used to be shipped with webcomponents.js v0 but was removed in the v1 version, hence the confusion and issue with IE11.

@ghost
Copy link

ghost commented Jul 5, 2017

I perfectly understand that errors happen, of course, anytime, anywhere... in this case, whatever the reason, you have to admit that it's really strange, that a whole team released such a giant error, and noone in testing realized this...
Anyway, apart from the full day of craziness I spent on this, I'm happy that in the end I could spot it! Only this! :-)
P.S.: by the way, I couldn't find the #1020 mentioned by @krumware :-( Someone could post the reference here! :-)

@stramel
Copy link
Contributor

stramel commented Jul 5, 2017

@jsilvermist working on getting documentation, perf profiling, and some fixes into the repo before we merge and tag.

Also, I think this should be closed since it IE11 and Edge just warn about the comment. Using the RegExp should fix the failing load on IE11.

@rpapeters
Copy link

Just updated polymer-cli to 1.3.1 and generated a fresh polymer-2-starter-kit app. Still seeing the same issue:
ie11_psk_error

@schlm3
Copy link

schlm3 commented Jul 12, 2017

Same here. Discovered this in 1.1.0 already and its still there in 1.3.1.
However, I can not see any negative impact on our polymer-1 application in IE except for the warning in the console.
Of course it is pretty awkward, that google improves the polymer-linter to check for correct syntax and at the same time introduces illegal syntax during build...

@ghost
Copy link

ghost commented Jul 12, 2017

Yes, Polymer 1... we were talking about the Polymer 2 starter kit, where you cannot even see the first page, screen in IE remains blank...

@krumware
Copy link
Contributor

@unionthugface iron-ajax now requires the promise polyfill instead of including it. Check out the iron-ajax repo PolymerElements/iron-ajax#280

@moodyjmz
Copy link

bower_components/shadycss/src/style-properties.js - I can see this being delivered as ES6 into IE11

And CustomElementInternals.js - which seems to be dynamically generated by polymer serve

This is cli v1.3.1

@JoceM
Copy link

JoceM commented Jul 24, 2017

Is there any updates about this ? This is pretty critical and strangely not widely discussed anywhere else. Folks reverting to Polymer 1 ?

@stramel
Copy link
Contributor

stramel commented Jul 24, 2017

This has been come a catch all issue and should be closed as the root issue has been addressed. The fix for the original issue is to switch from using new URL to a regex based lookup as pointed out here Polymer/polymer-starter-kit#1020.

@FredKSchott @abdonrd @LawrenceMok Can we close this?

@rpapeters
Copy link

rpapeters commented Jul 24, 2017 via email

@stramel
Copy link
Contributor

stramel commented Jul 24, 2017

polymer-cli master seems to be pulling the ^3.0.0 version of PSK. Which appears to not to have the latest tagged yet. So maybe this stays open until it is tagged? Follow status of tag here: Polymer/polymer-starter-kit#1038

@abdonrd @justinfagnani

@abdonrd
Copy link
Contributor

abdonrd commented Jul 24, 2017

@stramel @rpapeters The problem is solved, but now we have to wait for a new PSK release. The Polymer CLI doesn't need to be updated, because it takes the last PSK tag.

@rpapeters
Copy link

rpapeters commented Jul 24, 2017 via email

@abdonrd
Copy link
Contributor

abdonrd commented Jul 24, 2017

@rpapeters you can download the PSK 2.0-preview branch.

@rpapeters
Copy link

rpapeters commented Jul 24, 2017 via email

@abdonrd
Copy link
Contributor

abdonrd commented Jul 24, 2017

@rpapeters No, I mean direct download:

https://github.com/PolymerElements/polymer-starter-kit/archive/2.0-preview.zip

Instead of:

polymer init polymer-2-starter-kit

@rpapeters
Copy link

rpapeters commented Jul 24, 2017 via email

@motss
Copy link

motss commented Jul 26, 2017

No luck in getting it serve without any issue on IE11. Edge was fine.

@abdonrd
Copy link
Contributor

abdonrd commented Jul 26, 2017

Released Polymer Starter Kit v3.1.0! 😄

@stramel
Copy link
Contributor

stramel commented Jul 26, 2017

Hooray!

@rpapeters
Copy link

rpapeters commented Jul 26, 2017 via email

@phun-ky
Copy link

phun-ky commented Aug 22, 2017

Still getting this issue in IE11 11.540.15063.0

@ralcar
Copy link

ralcar commented Aug 23, 2017

Same here, updated to the latest Polymer CLI, created a fresh PSK 2 with PRPL pattern. Built to es5-bundled, and pushed it to a firebase hosting with firebase-tools.

Runs fine in chrome/firefox, but IE and Edge doesnt load. IE loads the app shell sometimes, but also takes a really long time before even showing anything.

@adamdoyle
Copy link

adamdoyle commented Dec 19, 2018

If it helps, the exclamation mark warning issue is not with the <!--! do not remove --> alone, but with the combination of both of the following:

<script>
if (!window.customElements) {
  document.write('<!--');
}
</script>
<!--! do not remove -->

In the case that window.customElements is undefined (e.g. on IE11 or Edge[?]), the resultant HTML is:

<!-- <!--! do not remove -->

(even the syntax highlighter isn't happy about it)

That comment-inside-a-comment violates §8.1.6(2) of the HTML5 Specification:

§8.1.6: Comments

Comments must have the following format:

  1. The string "<!--"

  2. Optionally, text, with the additional restriction that the text must not start with the string ">", nor start with the string "->", nor contain the strings "", or "--!>", nor end with the string "<!-".

  3. The string "-->"

Note:
The text is allowed to end with the string "<!", as in <!--My favorite operators are > and <!-->.

Here are three pages on Glitch demonstrating the different parts of the problem:

  1. No warnings: Only the <!--! do not remove --> (view in browser / edit)
  2. Warnings: The script that adds the <!--, as well as the <!--! do not remove --> (view in browser / edit)
  3. Warnings: The resultant bad HTML, hardcoded without Javascript (view in browser / edit)

It seems IE variants emit a warning, but Chrome stays silent. (even when it's hardcoded, as in example 3, above)

@adamdoyle
Copy link

If it can use Javascript to insert the opening comment, is there a reason it can't just use the Javascript to insert the entire <script> tag?

@stale
Copy link

stale bot commented Mar 3, 2020

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.

@stale stale bot added the wontfix label Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests