martok / palefill

Inject Polyfills for various web technologies into pages requiring them
https://martok.github.io/palefill/
Mozilla Public License 2.0
79 stars 9 forks source link

[GitHub] TypeError: v is not a constructor #59

Closed dirkf closed 1 year ago

dirkf commented 1 year ago

Lots of GitHub functionality is broken now in SeaMonkey 2.53.14 by this:

TypeError: v is not a constructor        relative-time-element.js:122:31
getFormattedDate                         relative-time-element.js:122:31
update                                   relative-time-element.js:298:23
attributeChangedCallback                 relative-time-element.js:283:13
a.prototype.attributeChangedCallback     custom-elements.min.js:19:172
<anonymous>                              custom-elements.min.js:36:418
update                                   relative-time-element.js:294:21
attributeChangedCallback                 relative-time-element.js:283:13
a.prototype.attributeChangedCallback     custom-elements.min.js:19:172
d                                        custom-elements.min.js:18:37
y.prototype.l/</<                        custom-elements.min.js:23:364
y.prototype.l/<                          custom-elements.min.js:22:67
y/this.f                                 custom-elements.min.js:19:863
y.prototype.l                            custom-elements.min.js:22:44
...

From my analysis below I can't say whether this is just a bug in GitHub's untested fallback when the browser doesn't have Intl.RelativeTimeFormat, or something weirder. For the moment I am polyfilling Intl.RelativeTimeFormat using a NoScript surrogate for <.githubassets.com to fix the issue, which favours the former explanation. If the same issue is affecting PM and it can be fixed in PF instead that would be great.

Analysis The problem code comes from https://github.com/github/relative-time-element. The original code is TypeScript but the bundled code sent by GH is minimised JS with JS source mapping. According to the SM debugger, the failing line is the apparently unproblematic ```js const relativeFormat = new RelativeTimeFormat(locale, { numeric: 'auto' }); ``` In the minimised JS, this is indeed `l = new v(k, {numeric: "auto"});`. The class `RelativeTimeFormat` is either `window.Intl.RelativeTimeFormat` (not defined in SM 2.53.14), or a "ponyfill": ```js ..., u = "Intl" in window && "RelativeTimeFormat", v = u ? Intl.RelativeTimeFormat : class RelativeTimeFormat { formatToParts() {...} resolvedOptions() {...} format(a, b) {...} }; ... ``` Although `u` must be `false`, somehow the ponyfill is being discarded. As I understand, since the ponyfill class has no constructor, the `new` args should just be discarded, rather than throwing an exception, like this Scratchpad output: ``` var v = class foo {}; new v(1,2,3); /* [object Object] */ ``` The ponyfill code: https://github.com/github/relative-time-element/raw/main/src/relative-time-ponyfill.ts The NoScript replacement using the rendering of that in the SM debugger: ```js if 'Intl' in window && !Intl.RelativeTimeFormat { class xRelativeTimeFormat { formatToParts() { return []; } resolvedOptions() { return { locale: 'en', style: 'long', numeric: 'auto', numberingSystem: 'nu' }; } format(value, unit) { if (value === 0) { switch (unit) { case 'year': case 'quarter': case 'month': case 'week': return `this ${unit}`; case 'day': return 'today'; case 'hour': case 'minute': return `in 0 ${unit}s`; case 'second': return 'now'; } } else if (value === 1) { switch (unit) { case 'year': case 'quarter': case 'month': case 'week': return `next ${unit}`; case 'day': return 'tomorrow'; case 'hour': case 'minute': case 'second': return `in 1 ${unit}`; } } else if (value === -1) { switch (unit) { case 'year': case 'quarter': case 'month': case 'week': return `last ${unit}`; case 'day': return 'yesterday'; case 'hour': case 'minute': case 'second': return `1 ${unit} ago`; } } else if (value > 1) { switch (unit) { case 'year': case 'quarter': case 'month': case 'week': case 'day': case 'hour': case 'minute': case 'second': return `in ${value} ${unit}s`; } } else if (value < -1) { switch (unit) { case 'year': case 'quarter': case 'month': case 'week': case 'day': case 'hour': case 'minute': case 'second': return `${-value} ${unit}s ago`; } } throw new RangeError(`Invalid unit argument for format() '${unit}'`); } } window.Intl.RelativeTimeFormat = xRelativeTimeFormat } ``` Personally I strongly disfavour anything that mucks around with date-time displays, especially if it's going to be continually updating all the date-time displays in all the open pages. They should use ISO 8601 date-time and put this "25 minutes ago" stuff in a tooltip that gets calculated when the tooltip is displayed.
Vangelis66 commented 1 year ago

I can't say whether this is just a bug in GitHub's untested fallback when the browser doesn't have Intl.RelativeTimeFormat, or something weirder. ... If the same issue is affecting PM

Well, I can't test latest official Pale Moon (v31.3.1) in my current OS but, by the looks of it, it, too, should at least exhibit #60; NB that this browser does carry native IRTF support, so, to use your wording 😉 , "something weirder" is taking place here...

martok commented 1 year ago

Pale Moon has native Intl.RelativeTimeFormat since 31.2.0 (2022-08-02) which "just works".

Looks like the SM build in question used an older platform?

dirkf commented 1 year ago

The stable releases of SM, now 2.53.x, are gradually getting back-ports. Although .53. may have been a reference to the original code having come from FF52ESR, this is the situation for 2.53.14:

SeaMonkey 2.53.14 uses the same backend as Firefox and contains the relevant Firefox 60.8 security fixes.

SeaMonkey 2.53.14 shares most parts of the mail and news code with Thunderbird. Please read the Thunderbird 60.8.0 release notes for specific security fixes in this release.

Additional important security fixes up to Current Firefox 91.11 and Thunderbird 91.11 ESR plus many enhancements have been backported. ...

Regarding this particular issue, on 14/11/2022 11:06, frg wrote:

Intl.RelativeTimeFormat is now in the 11/13 and up 2.53.16b1 pre builds. Please test. Still missing https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat/formatToParts which came with Firefox 70.

But apparently a dummy formatToParts() that returns an empty list is good enough for GH today: at least, that's what's in my polyfill.

As it's not a PM issue now and soon won't be for SM, let's close the issue.

martok commented 1 year ago

I'm confused. How does that platform work? I thought Fx57 got Electrolysis to the point where non-WebExtension addons were ripped out. Did the SM project somehow... not do that?

dirkf commented 1 year ago

Hence the reference to "backend". The "frontend" is, AIUI, the SM2 from the last common release with later stuff back-ported, like new JS syntax and in this case (2.53.16) Intl.RelativeTimeFormat.

For extensions the aim is to support WebExtension without unsupporting classic. This has just got to the point of language packs. I guess dictionaries are next. Somewhere down a very long line, MFv3.

SM 2.53 is pretty much a single process. It would be good for 32-bit to get each element of the suite in a different address space and for reliability to have separate processes so that crashing the browser doesn't crash the email. I don't think that's a priority now.

Vangelis66 commented 1 year ago

Pale Moon has native Intl.RelativeTimeFormat since 31.2.0 (2022-08-02) which "just works".

Yes, latest UXP does pass successfully the test at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/RelativeTimeFormat

> "in 3 qtrs."
> "1 day ago"
> "pasado mañana"

However, dirkf in his analysis above wrote that:

The problem code comes from https://github.com/github/relative-time-element.

... and on that repo it's mentioned that:

This element uses the Intl.DateTimeFormat & Intl.RelativeTimeFormat APIs, which are supported by all modern JS engines.

When the test over at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat is performed with latest UXP, I only get 2 out of 3 expected results:

> "12/20/2020"
> "20/12/2020"
> "20/12/2020"

FWIW, Chromium 86 (fork) gets 3 out of 3:

> "12/20/2020"
> "20/12/2020"
> "Sunday, 20 December 2020 at 14:23:16 GMT+11"

@martok , what do you make of this?

Lastly, have you been able thus far to pinpoint what GitHub changed in their served code/scripts on Nov 7th, that broke palefill-1.23 and caused #59 on SM-2.53.14 and #60 on UXP ? Many thanks 😄

dirkf commented 1 year ago

See the "analysis" in my original post. tldr; GH added new code using Intl.RelativeTimeFormat with a broken fallback.

martok commented 1 year ago

Hence the reference to "backend". The "frontend" is, AIUI, the SM2 from the last common release with later stuff back-ported, like new JS syntax and in this case (2.53.16) Intl.RelativeTimeFormat.

Interesting. Sounds like UXP, Waterfox and Seamonkey went wildly different ways.

Well then, looks like there is not much to do here if we already know the next Seamonkey release will have it natively.

The new issue of DateStyleFormat is now reported at UXP#2046.