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

'Undefined' module header on safari < 11 #1897

Closed
sejka opened this issue Jan 23, 2020 · 8 comments
Closed

'Undefined' module header on safari < 11 #1897

sejka opened this issue Jan 23, 2020 · 8 comments
Labels

Comments

@sejka
Copy link
Contributor

sejka commented Jan 23, 2020

Hey guys,
So i use MM as server only set up in a docker container and an old ipad as a screen. After last couple updates i noticed that my setup is all messed up with UNDEFINED headers above every module. So i check it on my laptop (chrome, w10) and everything is fine. Then i checked it on an iPhone (current ios) and also everything is fine. Thought this might be browser support issue so i tried browserstack.com for legacy safari versions and sure enough:
Safari 11.1:
image

Safari 10.1:
image

Safari 9.1:
image

I don't know if thats really a large issue for the whole community or how much we want to support older browsers but sure enough it worked fine circa december 2019. Thanks!

@sejka
Copy link
Contributor Author

sejka commented Jan 23, 2020

i think that's the one d8b7292#diff-77ab163850ef0fa98d6a9ee9fb6bbcaf

@sejka
Copy link
Contributor Author

sejka commented Jan 23, 2020

@tbouron is it possible to keep dynamic headers without adding them to the DOM and then hiding them?

@tbouron
Copy link
Contributor

tbouron commented Jan 23, 2020

@sejka With the currently way the code handles this, I'm afraid not. The wrapper DOM for the headers is only generated when the module is first initialised, then whatever comes from getHeader() is added to this wrapper. See the entire explanation on the PR description: #1838

It could be changed of course, but I didn't want to introduce too much changes. Hiding the wrapper DOM if getHeaders() returns undefined or an empty string seemed like a good compromise.

@sejka
Copy link
Contributor Author

sejka commented Jan 23, 2020

i see, i just started remote debugging on ipad and can see that style="display: none;" isn't even attached to the header for some weird reason :/.
image
And if i add it manually it works as expected.
Additionally i created css class called .hidden with simple display: none element and when I'm adding and removing that class instead of style element it works as expected. What do you think about that solution @tbouron?
Thanks!

@goszczynskip
Copy link

HTMLElement.style is supported from safari 11. https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/style

Maybe adding some polyfills or using setAttribute will work.

@tbouron
Copy link
Contributor

tbouron commented Jan 23, 2020

@sejka clearly the style isn't in a DOM which would suggest that there is a bug my code. Don't think we need a polyfill, inline styling this is pretty low level.

I would be interested to know what the getHeaders() function of this module returns with your configuration.

@sejka
Copy link
Contributor Author

sejka commented Jan 23, 2020

@tbouron just submitted pr, code is fine it's just not compatible with safari < 11
https://caniuse.com/#feat=mdn-api_htmlelement_style
check out the pr
Thanks!

@stale
Copy link

stale bot commented Mar 23, 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.

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

No branches or pull requests

3 participants