Page MenuHomePhabricator

MobileFrontend should not use moment.js for just one use case in the editor
Closed, ResolvedPublic

Description

Moment.js is quite a large library (70K gzipped) and is used only in a single line (though, loaded asynchronously).

It is currently only loaded in the mobile editor when a user is blocked
src/mobile.editor.overlay/EditorOverlayBase.js

Event Timeline

Change 513233 had a related patch set uploaded (by Simon04; owner: Simon04):
[mediawiki/extensions/MobileFrontend@master] Replace moment.js with Date.toLocaleString

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

Jdlrobson added a subscriber: Jdlrobson.

There is a problem with the patch as written. I think this should be the editing teams call as I'd hope long term that all editor code would live inside VisualEditor or one of their projects and I'm not sure whether moment.js is part of that. I don't think this is Reading-Web's domain, but let me know if it should be.

@Jdlrobson after checking with the team the Editing-team is responsible for this task, however, it isn't high on priority list at this time.

Moment is mostly needed to display "relative" times, as noted on the patch by Jon:

My understanding is that toLocaleString won't work here as moment is generating messages such as "a few seconds". So really this becomes a question of how useful that is, and whether it's worth the bloat that comes with moment.js

Personally I wouldn't mind removing the relative time, and only leaving the absolute expiration date. On desktop only the absolute expiration date is shown.


Historical note: the current code using Moment was added by me in rEMFR7b64322a2fdc: Add fancy block info popups from Minerva to MobileFrontend, replacing earlier implementation by @dbarratt in rSMINf51cf7db7b01: Use a Drawer for Block Notices that formatted dates in PHP code.

We only used them for registered users, but MobileFrontend's EditorOverlay code can use them for anonymous users as well.

(…) In Minerva we did this in PHP code and put it in mw.config(), but we can't do it for anonymous users, because that ends up in page HTML and is cached.

Formatting expiry time and duration is done using Moment.js rather than the Language class methods and custom code. (…)

Also, we could go back to rendering the relative expiry time in PHP – this could happen in ApiBlockInfoTrait (in MW core), which should allow it to be returned by API queries used by both editors (visual and wikitext). This didn't exist when David's original version was written (and VE didn't display block notices back then, now it does).

Change 513233 abandoned by Jdlrobson:
Replace moment.js with Date.toLocaleString

Reason:
Please restore once we've got consensus on the right approach on the ticket. Thanks!

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

Jdlrobson renamed this task from MobileFrontend should not use moment.js for a single line to MobileFrontend should not use moment.js for a single line in its editor.Jul 24 2020, 3:31 PM
Jdlrobson added a project: Editing-team.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a project: Performance Issue.
Jdlrobson updated the task description. (Show Details)
Jdlrobson moved this task from Team: web to Team:Editing on the MobileFrontend board.

Further motivation for doing this task is that the project is now considered to be in maintenance mode: https://momentjs.com/docs/#/-project-status/ (thanks @DLynch)

Personally I wouldn't mind removing the relative time, and only leaving the absolute expiration date.

This was my original intent in https://gerrit.wikimedia.org/r/513233. One would have to replace duration with expiry in src/mobile.editor.overlay/BlockMessageDetails.js. Does it make sense to amend my patch?

Also, we could go back to rendering the relative expiry time in PHP – this could happen in ApiBlockInfoTrait

A different approach could be using getTimeAgoDelta from src/mobile.startup/time.js (as is being done for getLastModifiedMessage; requiring a bunch of new i18n strings).

Removing task assignee due to inactivity, as this open task has been assigned for more than two years (see emails sent to assignee on May26 and Jun17, and T270544). Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be very welcome!

(See https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator.)

Esanders renamed this task from MobileFrontend should not use moment.js for a single line in its editor to MobileFrontend should not use moment.js for just one use case in the editor.Dec 2 2021, 2:24 PM

Change 743273 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/core@master] ApiBlockInfoTrait: Add formatted and relative times

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

Change 743274 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/MobileFrontend@master] Remove moment.js, use formatted timestamps from the API

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

Change 743273 merged by jenkins-bot:

[mediawiki/core@master] ApiBlockInfoTrait: Add formatted and relative times

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

Change 743274 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Remove moment.js, use formatted timestamps from the API

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

The new relative timestamps depend on the cldr extension (which is installed on all Wikimedia wikis). When it's not installed, absolute timestamps will be shown. Just noting for anyone testing this change locally, because I was confused by this myself at first.

matmarex edited projects, added Editing QA; removed Patch-For-Review.

QA steps: Try editing a page on mobile while blocked, ensure that the block expiration is shown as a relative time (e.g. "in 2 days") rather than absolute time (a date).

This @matmarex for the QA steps note. The block expiration is shown in relative time as expected

IMG_3103.PNG (2×1 px, 488 KB)

IMG_3102.PNG (2×1 px, 293 KB)

IMG_3101.PNG (2×1 px, 291 KB)

ppelberg claimed this task.