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

Unicode detection in Chrome 76 #2468

Closed
liviakuenzli opened this issue Sep 5, 2019 · 14 comments
Closed

Unicode detection in Chrome 76 #2468

liviakuenzli opened this issue Sep 5, 2019 · 14 comments

Comments

@liviakuenzli
Copy link

I'm using Google Chrome version 76.0.3809.132 (64-Bit) and modernizr 3.6.0 with Unicode-Character detection (https://modernizr.com/download/?-unicode-setclasses)

And modernizr is telling me that unicode is not 'supported'.
I tried debugging a bit and checked missingGlyph.offsetWidth and star.offsetWidth and they really seem to be the same now... I tried another unicode symbol (I randomly picked द) and this one seems to have a different offsetWidth.

Funnily enough I use the same chrome version on my Android and it does not seem to have the same problem, but I reproduced it on multiple (windows) computers.

I think for now I will just remove this check, but I just wanted to let you know that there might be a problem. Sadly I don't know how to fix it.

@shockthetoast
Copy link

shockthetoast commented Sep 5, 2019

We have run into the exact same issue. After updating to 3.7.1 the issue remains. Our current solution is also to remove the unicode checks - hopefully a browser that passes any of the others has unicode support.

It worked fine in 76.0.3809.100, and failed for the first time (on release) in 76.0.3809.132.

I actually ran into this for the first time when testing the developer preview of Microsoft Edge, which is switching to use Chromium for their rendering engine. When it failed there I did a little digging and ran into the same offsetWidth issues as @liviascapin did. However, I naively thought it was just an issue Microsoft had introduced. It turns out that Edge must have been using a pre-release version of Chromium with this change. I submitted an issue in Edge, but I did so using its feedback reporting tools that basically send info into the aether, so of course I never received a response.

It's interesting to note that this release was supposed to only be to fix some security bugs. So either something else slipped in, or one of the security fixes affected rendering.

Edit: I should note that I discovered the issue in Edge at least a few months back, so there is a chance it was in there before these security issues were addressed. Perhaps some other changes leaked through?

@rejas
Copy link
Member

rejas commented Sep 8, 2019

Thx for your input you two. I tried to fix this in #2470 could you try it out and report back if that fixes it for you too?

@shockthetoast
Copy link

@rejas It's still failing for us. Interestingly, if I add some console output, Chrome shows the offsetWidth for both characters as 2250. If I run it in Firefox, I get the offset widths as 883 for missingGlyph and 1820 for the unicode comet character.

My first thought was that Chrome is setting some maximum offsetWidth, but when comparing with Firefox it's reporting an offsetWidth many times larger - more than seems to be reasonably explained by quirks in the browsers' font handling differences.

Perhaps of note is the fact that if I also output the scrollWidth of both elements, Firefox shows the offsetWidth and scrollWidth as being identical, but Chrome reports a scrollWidth of 0. Perhaps Chrome has introduced some rendering delay that's preventing both sizes from being accurately calculated as quickly as necessary? Or this may just be a known browser difference of which I was unaware.

@shockthetoast
Copy link

This is an interesting development. I've created a fiddle to reproduce the Modernizr test.

https://jsfiddle.net/shockthetoast/fLnr7azb/

In Firefox it shows a placeholder characters, then the comet character.
In Chrome it shows two placeholder characters.

If I only append the comet glyph, it's fine. But it seems that when the invalid and valid characters are both appended, it treats them both as invalid. If I copy the output somewhere else, the second character becomes the comet character. So the actual character is there, it's just not rendering properly.

This seems to confirm that it's actually a bug in Chrome, and not intended behavior.

@shockthetoast
Copy link

I've submitted a bug report to Google. If anyone with this issue could star it, it might help us get some attention on the problem:
https://bugs.chromium.org/p/chromium/issues/detail?id=1002338

I was able to get the reproduction steps down to an HTML file like the following. It seems no Javascript is even required, merely having the two character added to the dom in that order causes the issue.

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="utf-8">
    <title>Test</title>
</head>

<body>&#5987;&#9732;</body>

</html>

@shockthetoast
Copy link

The good news is that the issue has been acknowledged as reproducible. They did make some font handling changes on purpose, but it looks like there is a bug that they will fix.

The bad news is that the priority was downgraded after declaring that the test modernizr uses isn't a good test and won't work... which is odd, since it's been in the library since 2012, and we've been successfully using it ourselves for about two and a half...

@shockthetoast
Copy link

@rejas Over at that bug request they are asking for some feedback from the Modernizr team. Is this feedback you could provide?

Thanks!

@rejas
Copy link
Member

rejas commented Sep 14, 2019

@shockthetoast took me a while but I replied :-)

@paulirish
Copy link
Member

@liviascapin btw do you currently rely on this test?

(I have the feeling like no one knows what this test indicates)

cc @ZeeDev @ZeeAgency .. do you recall what the motivation behind this test was? was there a browser who failed it at the time?

@liviakuenzli
Copy link
Author

We did use the test (we just added some tests to our site and if any of them fail we suggest the user download another browser). But, since it's broken, I temporarily removed the test from the list of required browser-features as all browsers should support unicode anyway. So we do not need the test necessarily.

I think even if this issue is resolved I might not add the test again since it doesn't seem to add much value and is more of an educated guess than a 'real' test (as stated by the chromium maintainer).

@paulirish
Copy link
Member

it doesn't seem to add much value and is more of an educated guess than a 'real' test (as stated by the chromium maintainer).

As someone who worked on Modernizr and now works on Chromium, yes, this is accurate. ;)

@shockthetoast
Copy link

@rejas Thanks!

I wonder if it should just be removed. I think the reason I started using it in the first place was that it was there. The fact that it was there probably made me wonder if there was a browser that might fail it, and since we need Unicode support we might as well test for it.

It doesn't help our current issue - content out in the wild on clients' systems now failing the test - but there isn't anything Modernizr can do that could fix that. Removing it might prevent someone in the future from having this issue.

I'm not sure what the policies are for deprecating tests in Modernizr. One option would be to update the test to always pass. It's not a good solution, but it might be a solution.

@rejas
Copy link
Member

rejas commented Sep 19, 2019

@shockthetoast as far as I know there is no policy (yet) for deprecating tests.

And yes, after reading @paulirish comment I would also put this test up for deprecation. If I find the time this weekend I whip up a proposal for that, maybe in the spirit of #2432 which I think is a candidate for deprecation too)

In short: a test like this would get removed from the config.all default build, would get marked accordingly in the source and docs and be removed completly with the next major version.

@rejas
Copy link
Member

rejas commented Oct 24, 2019

Closing this sicne we deprecated the test now

@rejas rejas closed this as completed Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants