search

m-lab / mlab-speedtest

Repository of the Interface and Project for speed.measurementlab.net
https://speed.measurementlab.net
Apache License 2.0
36 stars 14 forks source link

updated Safari bug description for team review #45

Closed critzo closed 3 years ago

critzo commented 3 years ago

Per team discussion, this PR updates the message presented to Safari users to communicate more clearly that this is an issue with Webkit that Safari uses for websockets, not an issue with the NDT test itself.

gfr10598 commented 3 years ago

Does onebox disable the scaling just for Safari, or for all browsers?

On Thu, Apr 22, 2021 at 11:12 AM robertodauria @.***> wrote:

@.**** commented on this pull request.

In app/measure/measure.html https://github.com/m-lab/mlab-speedtest/pull/45#discussion_r618493195:

@@ -1,8 +1,11 @@

-

We're sorry, the Safari browser is not compatible with the NDT - test. Please use a different browser such as Chrome or Firefox instead.

+

We're sorry, but M-Lab has identified bugs with Safari's websockets implementation that + results in innacurate results when using the browser to run the NDT test + (both ndt7 and ndt5 versions). Please use a different browser such as Chrome + or Firefox instead. For more information, please see this issue.

The download test is slow I believe due to a lot of garbage collection happening while receiving data, I don't think it can be mitigated. Nothing like that happens on other browser engines.

The upload test bug, which causes the client to crash after reporting an insanely high upload rate for the first 1-2 measurements (0.5s), can be mitigated by disabling message size scaling and reducing desiredBuffer, which is what the OneBox client does.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/m-lab/mlab-speedtest/pull/45#discussion_r618493195, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE7SM6S225JTKBIH3J4Y5YDTKA4GRANCNFSM43K6M5IA .

-- Greg Russell / Measurement-Lab https://memegen.googleplex.com/4558349824688128

critzo commented 3 years ago

@laiyi-ohlsen Would you have a moment to look at this text change? If so we can get it approved and push out a new release.

critzo commented 3 years ago

@gfr thanks for your review. If we could move the discussion about how OneBox deals with Safari over to Slack or a new issue, that will help us focus here on updating the messaging for our site users about Safari.

robertodauria commented 3 years ago

I believe this PR is now superseded by https://github.com/m-lab/mlab-speedtest/pull/46, which enables Safari again.