Page MenuHomePhabricator
Create Task
Maniphest T185948

Update web UI for increased comment length
Closed, ResolvedPublic
Actions

Assigned To
Anomie
Authored By
Anomie
Jan 29 2018, 9:32 PM
Tags
Referenced Files
None
Subscribers
Aklapper
Cirdan
Framawiki
gerritbot
He7d3r
Jack_who_built_the_house
Jdforrester-WMF
View All 11 Subscribers
Tokens
"Doubloon" token, awarded by Liuxinyu970226."Doubloon" token, awarded by RandomDSdevel.

Description

We will soon be able to start turning on the refactored comment storage, allowing comments of up to 1000 Unicode characters. This will be usable by API clients immediately. But everything in the web UI that applies a limit is still going to be applying the old limit of 255 bytes.

When the server-side $wgCommentTableSchemaMigrationStage variable is set to MIGRATION_OLD (0), frontend code should continue to apply the 255-byte limit. When that variable is set to any other value (other expected values are 1, 2, and 3), frontend code should limit things to 1000 Unicode characters. Note that JavaScript and HTML5 maxlength work with UTF-16 code points, e.g. "😭".length returns 2 but MediaWiki will count it as only 1; see also T180497: OO.ui.TextInputWidget's maxLength option limits by UTF-16 code units, not characters as documented.

Note that $wgCommentTableSchemaMigrationStage is temporary, so it should be made easy to search for in relevant code for future removal; naming variables to contain "CommentTableSchemaMigrationStage" or including that string in nearby comments is recommended. Extensions not using "compatibility policy = rel" may want to treat it as 0 when unset and the CommentStore class does not exist, and set to MIGRATION_NEW (3) when unset and the CommentStore class exists.

Places that may need attention for this task include:

  • The edit summary in the edit form, including the JavaScript that adds the byte counter.
  • The edit summary in VE, if it uses different code.
  • The reason field in the protection form.
  • The reason field in the page move form.
  • The reason field in the page deletion form.
  • The reason field in the file deletion form.
  • The reason field in the page undeletion form.
  • The reason field in the revision deletion form.
  • The reason field in the block form.
  • The reason field in the user rights change form.
  • Anything else that creates an entry in the logging table with a user-entered log_comment.

Note that extensions storing comments in their own tables don't need updating for this task, but may want to be updated to take advantage of CommentStore themselves.

Details

SubjectRepoBranchLines +/-
Update more forms to limit comments by codepoints rather than bytesmediawiki/coremaster+312 -51
Allow limiting comment length by characters rather than bytes in JSmediawiki/coremaster+214 -82
ve.ui.MWSaveDialog: Allow limiting comment length by characters rather than bytesmediawiki/extensions/VisualEditormaster+35 -17
Customize query in gerrit

Related Objects
Search...

Event Timeline

Anomie created this task.Jan 29 2018, 9:32 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 29 2018, 9:32 PM
Anomie added a subtask: T180497: OO.ui.TextInputWidget's maxLength option limits by UTF-16 code units, not characters as documented.Jan 29 2018, 9:32 PM
Anomie mentioned this in T180497: OO.ui.TextInputWidget's maxLength option limits by UTF-16 code units, not characters as documented.
Framawiki subscribed.Jan 29 2018, 9:38 PM
matmarex subscribed.Feb 5 2018, 5:23 PM

Is there a reason why CommentStore::COMMENT_CHARACTER_LIMIT (the magical "1000" value) is a constant rather than a configuration variable? We should probably get that value into mw.config, rather than change all the hardcoded 255s to hardcoded 1000s.

By "Unicode characters", do you mean "Unicode codepoints"? A "character" is a term that is notoriously hard to define.

Do you think it's important to make the limit 1000 codepoints, rather than 1000 UTF-16 code units? If we made it count UTF-16 code units, we could remove (or at least, stop using) all the code counting byte length, rather than having to adapt it to count codepoint length (or probably, duplicate it).

(…eh, we probably should make it count codepoints, just to make our software excellent, plus sooner or later someone is probably going to want a thousand emojis)

Anomie added a comment.EditedFeb 5 2018, 5:53 PM
In T185948#3946663, @matmarex wrote:

Is there a reason why CommentStore::COMMENT_CHARACTER_LIMIT (the magical "1000" value) is a constant rather than a configuration variable?

Because no one made a case for it to be configurable.

We should probably get that value into mw.config, rather than change all the hardcoded 255s to hardcoded 1000s.

I note that there's no need for it to be a configuration variable to be in mw.config, ResourceLoaderStartUpModule doesn't care how the value is determined.

By "Unicode characters", do you mean "Unicode codepoints"? A "character" is a term that is notoriously hard to define.

Yes. And true enough.

Do you think it's important to make the limit 1000 codepoints, rather than 1000 UTF-16 code units? If we made it count UTF-16 code units, we could remove (or at least, stop using) all the code counting byte length, rather than having to adapt it to count codepoint length (or probably, duplicate it).

Well, the limit already is 1000 codepoints in the code that has been merged (and already activated on Beta Cluster). In PHP we have all strings encoded as UTF-8 and mb_strlen() reports the length in codepoints. To measure the length in UTF-16 code units, we'd have to convert the string to that encoding and then use half the byte length.

And while "codepoints" will still be confusing to end users for situations with combining characters, doing code units will additionally be confusing with generic emojis or languages with characters outside the BMP. You'd still want the counter to indicate that.

matmarex closed subtask T180497: OO.ui.TextInputWidget's maxLength option limits by UTF-16 code units, not characters as documented as Resolved.Feb 5 2018, 8:50 PM
gerritbot subscribed.Feb 8 2018, 9:38 PM

Change 409168 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Allow limiting comment length by characters rather than bytes in JS

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

gerritbot added a project: Patch-For-Review.Feb 8 2018, 9:38 PM
matmarex added a comment.Feb 8 2018, 9:41 PM

This commit implements the "backend" part of the changes, and only updates the edit form and the protection form, as an example. Assuming we agree with the approach I took there, the same update can be easily done for the other instances, but I have no plans to work on that myself right now.

Anomie mentioned this in T166733: Deploy refactored comment storage.Feb 8 2018, 10:12 PM
• MichaelSchoenitzer_WMDE subscribed.Feb 8 2018, 11:26 PM
Jack_who_built_the_house subscribed.Feb 9 2018, 1:23 AM
Liuxinyu970226 added a project: User-notice.Feb 16 2018, 6:57 AM
Liuxinyu970226 moved this task from To Triage to In current Tech/News draft on the User-notice board.
Liuxinyu970226 subscribed.
Johan moved this task from In current Tech/News draft to Already announced/Archive on the User-notice board.Feb 21 2018, 11:53 AM
He7d3r subscribed.Feb 21 2018, 1:12 PM
RandomDSdevel awarded a token.Feb 23 2018, 2:07 AM
gerritbot added a comment.Feb 23 2018, 10:24 PM

Change 409168 merged by jenkins-bot:
[mediawiki/core@master] Allow limiting comment length by characters rather than bytes in JS

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

Jdforrester-WMF removed a project: Patch-For-Review.Feb 23 2018, 10:40 PM
Jdforrester-WMF updated the task description. (Show Details)
ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-02-27 (1.31.0-wmf.23)).Feb 23 2018, 11:00 PM
Anomie updated the task description. (Show Details)Feb 26 2018, 3:12 PM
gerritbot added a comment.Feb 26 2018, 6:17 PM

Change 414732 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] Update more forms to limit comments by codepoints rather than bytes

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

gerritbot added a project: Patch-For-Review.Feb 26 2018, 6:17 PM
gerritbot added a comment.Feb 27 2018, 9:38 PM

Change 415159 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] ve.ui.MWSaveDialog: Allow limiting comment length by characters rather than bytes

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

Jdforrester-WMF removed a project: Patch-For-Review.Feb 27 2018, 10:00 PM
Jdforrester-WMF updated the task description. (Show Details)
Anomie added a project: Patch-For-Review.Feb 27 2018, 10:04 PM

I20d11d7c still needs review, why remove the tag?

gerritbot added a comment.Feb 27 2018, 10:06 PM

Change 415159 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] ve.ui.MWSaveDialog: Allow limiting comment length by characters rather than bytes

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

Jdforrester-WMF subscribed.Feb 27 2018, 10:06 PM

Sorry, thought I had +2'ed that but clearly I was imagining it. Will (re?-)review.

Jdforrester-WMF updated the task description. (Show Details)Feb 27 2018, 10:21 PM
gerritbot added a comment.Feb 27 2018, 10:27 PM

Change 414732 merged by jenkins-bot:
[mediawiki/core@master] Update more forms to limit comments by codepoints rather than bytes

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

Reedy mentioned this in T188460: Special:EditTags limits the justification to length-155; consider changing this.Feb 27 2018, 11:18 PM
Liuxinyu970226 awarded a token.Feb 28 2018, 4:22 AM
Jdforrester-WMF mentioned this in T100161: Convert all of MediaWiki core to OOUI PHP (tracking).Mar 2 2018, 1:02 AM
jcrespo mentioned this in T6715: Allow comments longer than 255 bytes.Mar 2 2018, 1:18 PM
MichaelSchoenitzer subscribed.Mar 3 2018, 6:39 PM

The 2018 Wikitext-Editor still shows the limit of 255 Characters.

Anomie added a comment.Mar 4 2018, 4:45 AM

It shows the new limit on the Beta Cluster, e.g. https://en.wikipedia.beta.wmflabs.org/wiki/. The patch for VE should be included in 1.31.0-wmf.24, see https://www.mediawiki.org/wiki/MediaWiki_1.31/Roadmap for the deployment schedule.

ReleaseTaggerBot edited projects, added MW-1.31-release-notes (WMF-deploy-2018-03-06 (1.31.0-wmf.24)); removed MW-1.31-release-notes (WMF-deploy-2018-02-27 (1.31.0-wmf.23)).Mar 6 2018, 10:01 AM
• Niharika subscribed.Mar 9 2018, 7:31 PM
matmarex mentioned this in T188798: Change the limit of the edit summary to 500 characters on all Wikimedia wikis.Mar 14 2018, 9:43 PM
Anomie closed this task as Resolved.Mar 19 2018, 9:04 PM
Anomie claimed this task.

Let's call this done, since nothing has shown up for the "any other stuff" checkbox.

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptMar 19 2018, 9:04 PM
Liuxinyu970226 unsubscribed.Mar 20 2018, 3:34 AM
Cirdan reopened this task as Open.May 15 2018, 2:41 PM
Cirdan subscribed.

@Anomie There is at least one commonly used comment field left T194588: Increase limit for edit summary in Flagged Revisions to COMMENT_CHARACTER_LIMIT

It might be almost as easy as the patch I suggested, but probably some more changes are required.

Anomie closed this task as Resolved.May 15 2018, 2:50 PM

Better to use that task for tracking that issue.

MGChecker added a project: MediaWiki-Comment-store.Nov 12 2018, 7:45 PM
Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:39 PM
DannyS712 mentioned this in T291730: Remove javascript config setting wgCommentByteLimit.Sep 25 2021, 7:59 AM
Ladsgroup edited projects, added User-notice-archive; removed User-notice.Aug 13 2022, 1:53 PM
Umherirrender mentioned this in T13543: Use CommentStore::COMMENT_CHARACTER_LIMIT everywhere.Jan 5 2023, 12:40 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL