Page MenuHomePhabricator

GrowthExperiments: undefined is not a constructor (evaluating 'new Intl.Locale(language,languageOptions)')
Open, Needs TriagePublicBUG REPORT

Description

We get about 2/day of this error (example). Some user agents:

Mozilla/5.0 (iPhone; CPU iPhone OS 13_6 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.1.2 Mobile/15E148 Safari/604.1
Mozilla/5.0 (iPad; CPU OS 12_5_5 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.1.2 Mobile/15E148 Safari/604.1
Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.1.2 Safari/605.1.15

Caniuse says Intl.locale is supported in Safari on iOS 14+. Grade A support covers Safari on iOS 9+. So, we should probably do something about this, but not sure what.

Related Objects

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
KStoller-WMF added a subscriber: KStoller-WMF.

Do we know what the end user experience is when this error occurs? Is it breaking localization or translations in some way?

Unless it's breaking the page in a significant way, it seems like it's occurring infrequently enough to consider this an edge case that doesn't need an immediate fix.

I'll move to Triaged, but open to pulling this in if engineers thing there is a quick fix.

Do we know what the end user experience is when this error occurs? Is it breaking localization or translations in some way?

Since the exception is for the computation of the longest streak dates text "Feb 24 - 26"; it's probably breaking the opening of the streak informational tooltip.

Unless it's breaking the page in a significant way, it seems like it's occurring infrequently enough to consider this an edge case that doesn't need an immediate fix.

Would be nice to fix although I agree maybe not immediately.

We could try to proxy the logic we use Intl for behind a feature detection check, eg: Intl !== 'undefined' and have a fallback logic for the number and date formatting. The number formatting can probably done manually and the date could rely on moment.js which is already loaded. Actually the only use case we use moment is to display "time from now" in the last edit score card eg: "a month ago". It's a bit overkill to pull the full library just for a single method. This feature is also covered by Intl.RelativeTimeFormat. We could have some util script localeUtils.js that loads moment just for the cases Intl is not available.

On the long run maybe we could try to use some of the polyfills from formatjs. This requires some work to manage formatjs modules through foreign resources and would probably fit better as core resource. Then we could create some module similar to web2017-polyfills.

What do you think @kostajh @Tgr ?

We use Intl in three places: editing streak popup text, start and end date on the editing activity bar chart, and (in mobile overview only) pageview count on the trend chart.
You can more or less simulate the error by running Intl.Locale = null on page load; the bar chart will be missing entirely, the popup will not pop up, for the pageview graph the number will be missing but the chart will work.

I'd go for minimum-effort graceful degradation: not showing the info icon, and not showing the recent edits charts and showing a non-compact number on the pageview chart. IMO a polyfill / alternative date formatting logic is not worth the effort for a rare browser and relatively minor functionality.

kostajh added a subscriber: Masumrezarock100.

I'd go for minimum-effort graceful degradation: not showing the info icon, and not showing the recent edits charts and showing a non-compact number on the pageview chart. I

That sounds good to me.

Change 883984 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/GrowthExperiments@master] User impact: add graceful fallbacks for browsers without Intl

https://gerrit.wikimedia.org/r/883984

We use Intl in three places: editing streak popup text, start and end date on the editing activity bar chart, and (in mobile overview only) pageview count on the trend chart.
You can more or less simulate the error by running Intl.Locale = null on page load; the bar chart will be missing entirely, the popup will not pop up, for the pageview graph the number will be missing but the chart will work.

Thanks for detailing all failing cases.

I'd go for minimum-effort graceful degradation: not showing the info icon, and not showing the recent edits charts and showing a non-compact number on the pageview chart. IMO a polyfill / alternative date formatting logic is not worth the effort for a rare browser and relatively minor functionality.

For the streak dates I think we can just remove the affected second paragraph of text inside the tooltip, if that makes sense. The recent activity section won't appear and the mobile overview will show non-compact numbers.

kostajh added a subscriber: nettrom_WMF.

@nettrom_WMF do you think we should log something in homepagemodule schema to indicate if the user's browser has Intl support? Options would be:

  1. in action_data for all impact module related events
  2. just once in action_data for the impression event
  3. a top level property, like has_intl
{
  "action": "impression",
  // like this?
  "has_intl": true,
  // ... or like this?
  "action_data": "timeframe_in_days=60;timeframe_edits_count=795;thanks_count=6;last_edit_timestamp=1674681041;longest_streak_days_count=87;top_articles_views_count=288939723;has_intl=true",
  "user_id": 1,
  "user_editcount": 850,
  "user_variant": "control",
  "module": "impact",
  "is_mobile": false,
  "mode": "desktop",
  "homepage_pageview_token": "",
  "state": "activated"
}

Users without Intl will see a different version of the impact module.

With IntlWithout Intl
image.png (1×1 px, 169 KB)
image.png (1×1 px, 127 KB)

The number of users we see without Intl capabilities is very low (~2 per day) so maybe it's not relevant for data analysis; I'm unsure if we'd see different patterns when the impact module is rolled out more widely, but I suspect probably not.

Change 883984 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] User impact: add graceful fallback for browsers without Intl

https://gerrit.wikimedia.org/r/883984

@nettrom_WMF do you think we should log something in homepagemodule schema to indicate if the user's browser has Intl support?

I don't think we need to log this. Based on the link to Caniuse in the task this is related to old browsers, and so I think this rare occurrence will become more rare as time progresses. Secondly, we're adding graceful fallback and so I don't think we're looking to make additional product decisions based on this data. And third, if we're concerned that this might be a more frequent occurrence on the wikis, we can go look at pageview data to get a sense of how widespread older browsers are on our wikis.

@nettrom_WMF do you think we should log something in homepagemodule schema to indicate if the user's browser has Intl support?

I don't think we need to log this. Based on the link to Caniuse in the task this is related to old browsers, and so I think this rare occurrence will become more rare as time progresses. Secondly, we're adding graceful fallback and so I don't think we're looking to make additional product decisions based on this data. And third, if we're concerned that this might be a more frequent occurrence on the wikis, we can go look at pageview data to get a sense of how widespread older browsers are on our wikis.

I agree. Thanks for your comments on this!