[go: up one dir, main page]

Page MenuHomePhabricator

MobileDiff drops whitespaces from edits
Closed, ResolvedPublic

Description

When an edit marker (ins/del) shown on Special:MobileDiff includes leading/trailing whitespace, the whitespace is not displayed, showing as if the whitespace is missing from the edit. See e.g. this randomly chosen diff with no space between the commas and the green “no” words.

The page source does contain the spaces, the HTML code reads For example,<ins> no</ins> researchers. However, the ins tag uses display: inline-block which causes the browser (tested in Firefox and Edge) to drop the leading whitespace. The desktop version uses explicit white-space: pre-wrap which prevents the problem. (Cf. the example in the desktop version.)

Developers

The spaces are inside the ins and del tags but are being dropped. I think all that's needed is a

ins, del { white-space: pre-wrap; }

or

ins, del { white-space: break-spaces; }

qa steps

QA Results

ACStatusDetails
1T243783#5856549

Related Objects

StatusSubtypeAssignedTask
OpenBUG REPORTNone
OpenNone
StalledNone
OpenNone
OpenNone
DuplicateNone
OpenFeatureNone
OpenFeatureNone
DuplicateNone
ResolvedNone
OpenNone
OpenNone
OpenFeatureNone
OpenNone
ResolvedNone
ResolvedNone
OpenFeatureNone
OpenNone
ResolvedHalfak
OpenNone
OpenNone
OpenFeatureNone
StalledNone
OpenNone
OpenNone
OpenNone
ResolvedPetrb
OpenNone
OpenNone
Resolvedtstarling
OpenNone
DeclinedNone
ResolvedBUG REPORTJdlrobson
ResolvedNone
Resolvedovasileva
Resolvedovasileva

Event Timeline

I am probably seeing this behavior too. But not so sure. Screenshots? CC @Jdlrobson and @alexhollender for reviewing this change.

More examples would be helpful but yes this doesn't look correct.

OK, but this happens all the time to me (I was surprised not to find a bug already reported), so I guess it should not be too difficult to come with some more examples:

  • It seems the problem occurs mainly when a word is removed or added with spaces on both sides, because e.g. here, the removed part is followed by a number, so the whitespace is left outside the marker, showing correctly.
  • E.g. in https://en.m.wikipedia.org/wiki/Special:MobileDiff/937967506...937967684, where “perfect” drops the leading space, while “number”, followed by a comma, keeps the space outside, showing correctly (OBTW why does MobileDiff uses ... while Diff uses /? Never mind.)

But I guess the point is not to modify the diffing algorithm, only to fix the CSS styling…?

And screenshots of e.g. the enwiki example:

MobileDiff:

image.png (175×279 px, 3 KB)

desktop Diff:

image.png (144×240 px, 2 KB)

The spaces are inside the ins and del tags but are being dropped. I think all that's needed is a
``
ins,del { white-space: pre-wrap;}

or

ins, del { white-space: break-spaces; }

The problem has been noticed on itwiki as well. Pre-wrap seems good to me.

Change 569389 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Make sure ins and del pre-wrap with respect to whitespace

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

Jdlrobson triaged this task as High priority.

Change 569389 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Make sure ins and del pre-wrap with respect to whitespace

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

The suggested fix is inline (pun intended) with the treatment for the desktop diff 👍

Edtadros subscribed.

Test Result

Status: ✅ PASS
OS: macOS Catalina
Browser: Chrome
Device: MBP
Emulated Device: iPhoneX

Test Artifact(s):

QA Steps

Pick a random page on https://en.m.wikipedia.beta.wmflabs.org/
add or remove a space from the page
click last modified bar at bottom of page and view diff
✅ AC1: confirm space is visible in diff

en.m.wikipedia.beta.wmflabs.org_wiki_Special_MobileDiff_412755(iPhone X).png (2×1 px, 225 KB)

looks good, resolving.