go-gitea / gitea

Git with a cup of tea! Painless self-hosted all-in-one software development service, including Git hosting, code review, team collaboration, package registry and CI/CD
https://gitea.com
MIT License
45.16k stars 5.49k forks source link

JavaScript error: RangeError: invalid value unit for option style #26525

Closed veita closed 1 year ago

veita commented 1 year ago

Description

Visit https://gitea.example.org/admin. The following error is shown on a red bar on the webpage:

JavaScript error: RangeError: invalid value unit for option style
(https://git.nezwerg.de/assets/js/webcomponents.js?v=1.20.2 @ 1:21550).

Gitea Version

1.20.2

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

git version 2.39.2

Operating System

Debian 12 Bookworm

How are you running Gitea?

Gitea binary downloaded from https://github.com/go-gitea/gitea/releases/download/ and run as a Systemd service.

Browser: Pale Moon 32.3.1.

Database

PostgreSQL

silverwind commented 1 year ago

Can you show a screenshot of the surrounding JS code when you click the error line number in the browser console (F12)?

wxiaoguang commented 1 year ago

The exception is thrown here:

https://github.com/github/relative-time-element/blob/d0da95b8c69bd22b76781d50d6143f7427844380/src/duration-format-ponyfill.ts#L113

What's your browser name/version and what's your locale (eg: run window.document.documentElement.lang in your browser console)?

(Hmm, I see Browser: Pale Moon 32.3.1, maybe it's a browser's bug)

And, are you using some browser plugins like translators? Does the error still exist in private window?

wxiaoguang commented 1 year ago

PaleMoon bug (incomplete Intl.NumberFormat support).

You could test it on this page https://github.github.io/relative-time-element/examples/ and fix the bug accordingly.

image

veita commented 1 year ago

https://repo.palemoon.org/MoonchildProductions/Pale-Moon/issues/1930

wolfbeast commented 1 year ago

It's not necessarily a "bug", it's just "yet another spec change" and "yet another tacked on API". We did lift our Intl support to the proper level recently, however we do not yet support the so-called Unified NumberFormat API which Gitea apparently now wants to use. i.e. not a bug, just another feature we'll have to implement.

Couldn't this have been implemented a bit more browser-agnostic in Gitea? I mean, not like "relative date formats" are a new thing, at all.

wxiaoguang commented 1 year ago

Before 1.20: the "relative date" is rendered by backend, it doesn't support i18n well.

1.20: by using GitHub's "relative-time" element (#23988) , most date/time/duration texts are rendered by browser.

wolfbeast commented 1 year ago

Would you consider keeping the backend implementation as a fallback, at least? I mean, just hard throwing an error is always worse than "not supporting i18n well".

(as an aside, GitHub has indicated they want to always be on the bleeding edge of new client APIs, so using that as a reference for "what to implement" may not always be a good idea...)

wxiaoguang commented 1 year ago

I am neutral for it, but I am not familiar with the "relative-time" change. Maybe the original author @yardenshoham could suggest.

wolfbeast commented 1 year ago

For the record, GitHub's relative duration implementation does seem to render correctly in Pale Moon.

wxiaoguang commented 1 year ago

Maybe they use some internal packages or they use some polyfill packages.

yardenshoham commented 1 year ago

Perhaps we could add some sort of polyfill for the missing NumberFormat API? I see a backend implementation as a step back in terms of evolution

wxiaoguang commented 1 year ago

Just get an (somewhat strange) idea, would PaleMoon like to pack some polyfill packages by default? Then many browser features could be used out-of-box, instead of waiting for a C++ native implemention. 😁

wolfbeast commented 1 year ago

Just get an (somewhat strange) idea, would PaleMoon like to pack some polyfill packages by default?

That's rather complex and potentially dangerous as we can't just inject scripts into content willy-nilly, especially not if they are part of the browser's internal resources (it breaks the chrome/content boundary); on top, blanked polyfilling will conflict with other websites that may polyfill things themselves. In isolation it might be a good idea but not on the open web, unfortunately. We have an open issue to implement this API now, but no immediate timeline as to when we'll get to it.

wxiaoguang commented 1 year ago

Made some tests, the minimal polyfill is like this (without i18n support):

    (function() {
        const old = Intl.NumberFormat;
        Intl.NumberFormat = function(locales, options) {
            if (options.style === 'unit') {
                return {
                    format: function(value) {
                        return ` ${value} ${options.unit}`;
                    }
                }
            }
            return old(locales, options);
        };
    })();

image

wxiaoguang commented 1 year ago

Add minimum polyfill to support "relative-time-element" in PaleMoon #26575

Does it look good or could there be better solutions?

yardenshoham commented 1 year ago

How about https://formatjs.io/docs/polyfills/intl-numberformat/?

wxiaoguang commented 1 year ago

See the FAQ in #26575

image

yardenshoham commented 1 year ago

I implemented https://formatjs.io/docs/polyfills/intl-numberformat/ with all locales, you are right the size will go from 60KB to 3MB, I wanted to only load the needed locale but that requires dynamic import which is asynchronous but we need it before we register the web component... Also we can't register the web component after the locale load because the HTML already has the relative-time