Fixes #341

[Schrank]

Fixes #341

[oliverklee]

This PR now has merge conflicts. Could you please resolve them and repush, @Schrank? Thanks!

Also, we should have a regression test (that passes with the fix and fails without it.)

[Schrank]

@oliverklee Happy to fix the merge conflict, but as I read the bug description it sounds like something broken with the libraries below PHP. I haven't tried but don't think writing a test is easy to achieve :-/ Therefore: Sorry, but I don't try it.

[dinamic]

@Schrank won't rebasing the PR towards the current master solve things?

The only adjustment needed is probably to add dependency on ext-mbstring in composer.json.

[Schrank]

@dinamic To avoid misunderstanging: I'm not blocking to help, conflict is resolved and mbstring is added!

I'm just denying to implement a test, because that feelss very heisen-buggy :-(

[dinamic]

@Schrank I maybe able to help with the unit test, if you like.

I'd need to reproduce the problem tho. Could you give me the steps needed with a docker setup, for example?

[coveralls]

coverage: 45.68%. remained the same
when pulling 49ee98d on Schrank:patch-1
into 1fad44a on MyIntervals:main.

[JakeQZ]

@dinamic, thanks for offering to help out. Have you found a similar problem?

I'd need to reproduce the problem tho.

That may be tricky, since some forensic evidence may have been lost in the midsts of time.

The report cites an E_NOTICE from iconv() which is not an exception. Perhaps mb_convert_encoding() does not give such a notice upon error, and it is thus irrelevant which is used, since both will fail (one more silentlty that the other).

The likely CSS (untested) would be something like

.smiley::after {
  content: "\1F642"
}

I did some digging and found this comment on Stack Overflow:

iconv shows this error message if either encoding specified does not exist... On Linux, iconv -l in a shell shows what character sets are supported

My thinking is that this is a server-specific issue, and there is no (current) justification for switching from iconv to mb_string.

Though if iconv is tied to an unreliable OS function, whilst mb_convert_encoding comes reliably packaged with PHP, the latter would be preferable - if that's the case.

@dinamic, if you'd like to create some tests, they would be very welcome. The CSSString class needs a corresponding TestCase... :))

With a decent set of tests, we would have a better idea of whether switching from iconv to mb_string would be viable and positive.

Also, we should have a regression test (that passes with the fix and fails without it.)

I don't think that's possible, since the issue is confuguration-dependent.

[Schrank]

@JakeQZ thanks for chiming in!