Page MenuHomePhabricator

Use monospace font (or editfont preference) for diffs
Closed, ResolvedPublic

Assigned To
None
Authored By
Esanders
Apr 16 2020, 2:15 PM
Referenced Files
None
Tokens
"Y So Serious" token, awarded by matej_suchanek."Pirate Logo" token, awarded by Josve05a."Like" token, awarded by taavi."Dislike" token, awarded by Nihlus."Like" token, awarded by OldUser01."Dislike" token, awarded by QEDK."Dislike" token, awarded by Kizule."Dislike" token, awarded by Aca."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

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}

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

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.

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).

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

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)

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.

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!

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

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?

In T250393#6115543, @ValeJappo wrote:

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.

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.

@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.

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?

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?

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

@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.

[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.

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

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.

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

@Esanders: Is there more to do in this task or can it be closed?

Currently the font has a pixel-based size (gerrit 593629).

Though, it has been been pinpointed later by gerrit 702094 (specifically, here).

Would it be possible to change this to a relative font size?

Use case: I'm working on scripts that load and embed remote pages, including diff pages. A relative font-size is applied to the container of the embedded page, so that its whole display size is reduced in the interface. But because of this fixed 13px size font size, the diff table doesn't scale down and stays very large.

ping @Esanders @Jdforrester-WMF

matmarex subscribed.

I guess this is resolved…