Page MenuHomePhabricator

Use monospace font (or editfont preference) for diffs
Open, Needs TriagePublic

Assigned To
None
Authored By
Esanders
Apr 16 2020, 2:15 PM
Tokens
"Y So Serious" token, awarded by matej_suchanek."Pirate Logo" token, awarded by Josve05a."Like" token, awarded by Majavah."Dislike" token, awarded by Nihlus."Like" token, awarded by OldUser01."Dislike" token, awarded by QEDK."Dislike" token, awarded by Kizule."Dislike" token, awarded by Acamicamacaraca."Like" token, awarded by Lofhi."Like" token, awarded by Tractopelle-jaune."Like" token, awarded by Thibaut120094."Dislike" token, awarded by Pppery.

Description

Following on from T223429, we always display wikitext in monospace (or whatever the user has set as their editfont preference), however the diffs are always shown in sans-serif.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 593629 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/core@master] Follow-up I07dd6f7: Fix font size in diff

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

Esanders added a comment.EditedApr 30 2020, 10:35 PM

There was an oversight in setting up the new classes that meant the diff text was 12.32px instead of the 13px the edit area uses. The above patch fixes that.

Change 593629 merged by jenkins-bot:
[mediawiki/core@master] Follow-up I07dd6f7: Fix font size in diff

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

Change 593632 had a related patch set uploaded (by Jforrester; owner: Esanders):
[mediawiki/core@wmf/1.35.0-wmf.30] Follow-up I07dd6f7: Fix font size in diff

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

Removing Community-consensus-needed as that is not the case.

Also, "I like it" / "I don't like it" comments are not helpful (software development is not a popularity contest). :) Thanks a lot!

We need a simple explanation of how to get back to the old view.

See https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)#Thursday_font_change what to add to your personal CSS:

.diff-editfont-monospace .diff-addedline, .diff-editfont-monospace .diff-deletedline, .diff-editfont-monospace .diff-context { font-family: sans-serif; }

Removing Community-consensus-needed as that is not the case.

Also, "I like it" / "I don't like it" comments are not helpful (software development is not a popularity contest). :) Thanks a lot!

-That's right! Let's have no more of this nonsense about giving the user what they want or need!

QEDK added a comment.May 2 2020, 6:48 AM

Removing Community-consensus-needed as that is not the case.

Also, "I like it" / "I don't like it" comments are not helpful (software development is not a popularity contest). :) Thanks a lot!

Typography and UI elements *are* on the basis of "I like it" and "I don't like it", most community members don't worry about underlying changes as long as what they see is consistent. The link you pointed to stated specifically that not all changes can be announced (I agree!) but also that the Phabricator boards are open and meant for feedback, that's what's happening here. Let's not stifle the discussion.
As I said before, this is about giving people the option to choose, an option they don't have yet. Since the change was made, the onus is on the developers to also give an option, that's all.

SD0001 removed a subscriber: SD0001.May 2 2020, 7:00 AM
hgzh added a subscriber: hgzh.May 2 2020, 10:34 AM

Has this been reverted? dewiki seems to have the proportional font back. In addition, diffs shown in unreviewed revisions of a page (FlaggedRevs) did not use the monospaced font even before today.

Has this been reverted? dewiki seems to have the proportional font back. In addition, diffs shown in unreviewed revisions of a page (FlaggedRevs) did not use the monospaced font even before today.

wmf.30 has been reverted back to group0 wikis and has been since Thursday due to issues unrelated. See https://versions.toolforge.org/ and {T249962#6097987}

MBH added a comment.May 2 2020, 10:40 AM

I agree with QEDK: this change should get consensus on big wikis or option in preferences to get this changes back.

Dvorapa added a subscriber: Dvorapa.EditedMay 2 2020, 11:48 AM

On cswiki this seems as reverted currently. I personally agree with the change, but better readable font should be used as the font used yesterday was barely readable. Weirdly enough the font was also different from 2010 source editor font (at least in Firefox 75), which seems like a bug in that reverted change.

QEDK added a comment.May 2 2020, 11:58 AM

On cswiki this seems as reverted currently. I personally agree with the change, but better readable font should be used as the font used yesterday was barely readable. Weirdly enough the font was also different from 2010 source editor font (at least in Firefox 75), which seems like a bug in that reverted change.

The 2010 source editor forces monospace it seems (just tested it out right now). Differences are sans-serif by default (which is the point of contention here basically).

Dvorapa added a comment.EditedMay 2 2020, 12:01 PM

I talk about the monospace font used in diffs yesterday: It was a different monospace font from what monospace font is used by 2010 source editor.

While this seems fine for source code based content, I'm not sure about diffs over Wikidata entities.

There should be seperate options for font in editing space and fonts in diffs, with the default being monospace and sans serif respectively

QEDK added a comment.May 3 2020, 7:18 AM

I talk about the monospace font used in diffs yesterday: It was a different monospace font from what monospace font is used by 2010 source editor.

Hmm, no idea about that. That's weird definitely.

There should be seperate options for font in editing space and fonts in diffs, with the default being monospace and sans serif respectively

Status quo should be maintained, yes. 👍

Change 593632 merged by jenkins-bot:
[mediawiki/core@wmf/1.35.0-wmf.30] Follow-up I07dd6f7: Fix font size in diff

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

Mentioned in SAL (#wikimedia-operations) [2020-05-04T15:43:00Z] <jforrester@deploy1001> Synchronized php-1.35.0-wmf.30/resources/src/mediawiki.diff.styles/diff.less: T250393 Follow-up I07dd6f7: Fix font size in diff (duration: 01m 05s)

He7d3r added a subscriber: He7d3r.May 4 2020, 5:49 PM
Naleksuh added a comment.EditedMay 4 2020, 6:25 PM

There should be seperate options for font in editing space and fonts in diffs, with the default being monospace and sans serif respectively

Or just ignore this and reinstate the change that everyone near-unanimously hated. Is it that hard to add a second option for both editing space and diffs? Please don't get into the habit of discord and such where every update makes the platform worse

I talk about the monospace font used in diffs yesterday: It was a different monospace font from what monospace font is used by 2010 source editor.

Seems resolved now.

Kizule awarded a token.May 5 2020, 8:57 PM
QEDK awarded a token.May 6 2020, 9:16 AM
Nihlus awarded a token.May 6 2020, 4:14 PM
GeoffreyT2000 added a subscriber: GeoffreyT2000.EditedMay 7 2020, 4:05 AM

I am going to submit revert patches for the three merged Gerrit patches, because people do not like the change.

Change 594824 had a related patch set uploaded (by GeoffreyT2000; owner: GeoffreyT2000):
[mediawiki/core@master] Revert "Follow-up I07dd6f7: Fix font size in diff"

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

Change 594825 had a related patch set uploaded (by GeoffreyT2000; owner: GeoffreyT2000):
[mediawiki/core@master] Revert "Follow-up I07dd6f78: Use correct font on live preview of changes"

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

Change 594826 had a related patch set uploaded (by GeoffreyT2000; owner: GeoffreyT2000):
[mediawiki/core@master] Revert "Apply editfont preference to diff rendering"

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

Can the team responsible for this change please update this tasks description and document:

  • Which team made the change?
  • Why was this font out of a sudden becoming a problem, after it was like this for I guess 15 years?
  • Who decided to go with this change, based on what input and arguments?
  • Have real-world user tests been made? How many users have been asked? What was the background of these users?

Personally, I refuse to check if the answers to these questions might be buried somewhere in the comments here or on Gerrit. Please properly document it. Thank you a lot!

Od1n added a subscriber: Od1n.May 7 2020, 7:36 AM

Personally, after some initial reluctance, I tend to prefer the monospace, but I had to tweak the font in my user CSS.
The font really has to be fine-tuned, and it certainly varies a lot depending on the OS/browser/etc.

Some notes:

  • Definitely, this has to be configurable independently from the edit zone font.
  • I use the the Live preview functionnality, and I noticed that when doing "Show changes" the font is still sans-serif.

Confirmed, Live preview does not use monospace font

Od1n added a comment.May 7 2020, 9:33 AM

Just as a reminder, the changeset has been reverted

Just as a reminder, the changeset has been reverted

The edit that add the monospace font has been reverted or the one that removes it?

Just as a reminder, the changeset has been reverted

The edit that add the monospace font has been reverted or the one that removes it?

Neither, the patches reverting the monospace font have been created but not merged.

Od1n added a comment.EditedMay 7 2020, 10:32 AM

Look just above, the font <del>has been</del> <ins>is going to be</ins> restored to sans-serif.

(So the Live preview functionnality doesn't need to be worked on. Just keep it in mind, in case of new font change attempt in the future.)

Personally, after some initial reluctance, I tend to prefer the monospace, but I had to tweak the font in my user CSS.
The font really has to be fine-tuned, and it certainly varies a lot depending on the OS/browser/etc.

Some notes:

  • Definitely, this has to be configurable independently from the edit zone font.
  • I use the the Live preview functionnality, and I noticed that when doing "Show changes" the font is still sans-serif.

Live preview was fixed in https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/593553/ which will go out with wmf31 today.

QEDK added a comment.EditedMay 7 2020, 3:40 PM

@Esanders Please read https://www.mediawiki.org/wiki/Gerrit/Code_review#Is_it_wanted

The very first question is whether the contribution is a good idea. If the contribution is not helpful or does not align with the direction and scope of the project, explain and provide feedback on better ideas.

Please do not use your -2 to enforce your choice like you did here: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/594826#message-8eec13afca3dbc093df195463c9bbf8cb415c91a (it's construed a misuse)
You also did not follow any steps at https://www.mediawiki.org/wiki/Gerrit/Code_review#Giving_negative_feedback :

Most importantly, give the feedback quickly. Don't just leave negative feedback to someone else or hope they aren't persistent enough to get their contribution accepted. Let them know where they can appeal your decision. For example, invite them to discuss the issue on mail:wikitech-l or on IRC.

Personally, after some initial reluctance, I tend to prefer the monospace, but I had to tweak the font in my user CSS.
The font really has to be fine-tuned, and it certainly varies a lot depending on the OS/browser/etc.

Some notes:

  • Definitely, this has to be configurable independently from the edit zone font.
  • I use the the Live preview functionnality, and I noticed that when doing "Show changes" the font is still sans-serif.

Live preview was fixed in https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/593553/ which will go out with wmf31 today.

Just a note that, thanks to yours truly, wmf.31 has been reverted back to testwiki only

So... I personally like the new font. But such changes MUST have the wider community consensus. As far as I can see, it has not even been announced that the font will be changed (on Village Pump or somewhere else). Give people what they want. Don't forget that without users there is no programming, there is no Wikipedia, no wikis.

Since opinions are divided, be rational and create an option by which each user can choose the font that suits him.

Note: The font change has been reverted on sr.wiki.

Johan added a subscriber: Johan.May 9 2020, 11:17 AM

It was announced in Tech News (https://meta.wikimedia.org/wiki/Tech/News/2020/17 with a link to this Phab ticket) which is the typical way most changes are announced. You can subscribe here, if you aren't subscribed already:
https://meta.wikimedia.org/wiki/Global_message_delivery/Targets/Tech_ambassadors

If ppl are really interested in undoing this, in my opinion, you need to do a better job at explaining why you have NOT set your editor font to a sans-serif font, leaving it monospace, yet require a sans-serif font in diffs.

Both are wikitext, both require careful reading when engaging with the wikicode. As such there was little reason to assume the request (T223429) by @SD0001, should not extend to all diffs. From a technical perspective that consistency makes sense (and definitely doesn't need an RFC before going through a BRD cycle). If there is a difference, that difference needs careful explaining (beyond, 'i don't like it'). If there isn't a difference, maybe just change your font setting in the preferences and be happy in BOTH interfaces ?

Also can people pls respect that MediaWiki cycles take weeks, not minutes please ? This is software and software is developed at a bit different pace than Wikipedia articles.

@TheDJ, not that there is anything wrong with what you and others say here. But it's like everybody is avoiding one – in my opinion – crucial fact: This font was not a bug. It was like this for possibly 15 years (from memory, hard to actually check). Shouldn't the fact it was not challenged and changed for so long tell us something?

QEDK added a comment.May 9 2020, 1:27 PM

If ppl are really interested in undoing this, in my opinion, you need to do a better job at explaining why you have NOT set your editor font to a sans-serif font, leaving it monospace, yet require a sans-serif font in diffs.

That's a good viewpoint, if there are any features you like, I would like to remove them and make you prove why you want them? Sounds good?

Pppery added a subscriber: Pppery.May 9 2020, 6:54 PM

I can imagine that this solved to some extent T15466: Design of diffs should be improved to indicate white space changes better. (Just guessing though.)

@QEDK: While disagreement is welcome, sarcasm is not helpful. Please see https://www.mediawiki.org/wiki/Bug_management/Phabricator_etiquette - thanks.

QEDK added a comment.EditedMay 10 2020, 3:41 PM

@QEDK: While disagreement is welcome, sarcasm is not helpful. Please see https://www.mediawiki.org/wiki/Bug_management/Phabricator_etiquette - thanks.

It is weirdly hypocritical that you would say "disagreement is welcome" when 1) none of the complaints were addressed 2) you do not address developers on not following Gerrit best practices.
Edit: I do not think there is literally any cost-to-benefit ratio of voicing my opinion here. I am unsubscribing myself from this, if I've offended anyone, I apologize. If anyone wants to read more, this was my last approach at trying to make my point: https://en.wikipedia.org/w/index.php?title=User_talk:Whatamidoing_(WMF)&oldid=955757767#Change Good luck.

QEDK removed a subscriber: QEDK.May 10 2020, 3:47 PM
SD0001 removed a subscriber: SD0001.May 10 2020, 3:52 PM

[off-topic]

In T250393#6122779, QEDK wrote:

While disagreement is welcome, sarcasm is not helpful. Please see https://www.mediawiki.org/wiki/Bug_management/Phabricator_etiquette - thanks.

It is weirdly hypocritical that you would say "disagreement is welcome" when 1) none of the complaints were addressed 2) you do not address developers on not following Gerrit best practices.

Well, I'm a bugwrangler in Phabricator and not a patchwrangler in Gerrit. This conversation takes place in Phabricator and not in Gerrit.
Commenting on one aspect does not mean that I must comment on all other aspects (even though I may share some opinions).

I have made requests for CR -2 (Code-Review -2) to be removed from the following three revert patches:

Please do the removal as soon as possible.

Od1n added a comment.May 29 2020, 12:21 AM

I would like to clarify, are you (the devs) going to keep or revert the switch to monospace?

thiemowmde added a comment.EditedMay 29 2020, 6:04 AM

I removed all -2 because there was zero (!) reason given for the -2, neither here nor in any of the patches. I don't think this is acceptable.

I'm also waiting for a response – really any response – to the question I asked 3 weeks (!) ago in T250393#6115246. Which product manager or team made this decision, based on what research? @Esanders is listed as a software engineer (contractor) in the engineering team. If this was done as part of your job, please poke the responsible product manager to respond here. Or was this done in a volunteer role?

Update: @Esanders added all -2 back, still without giving a reason.

stjn removed a subscriber: stjn.May 29 2020, 7:16 PM

Change 594824 abandoned by GeoffreyT2000:
Revert "Follow-up I07dd6f7: Fix font size in diff"

Reason:
Per Esanders.

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

Change 594825 abandoned by GeoffreyT2000:
Revert "Follow-up I07dd6f78: Use correct font on live preview of changes"

Reason:
Per Esanders.

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

Change 594826 abandoned by GeoffreyT2000:
Revert "Apply editfont preference to diff rendering"

Reason:
Per Esanders.

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