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
Declineddchen
OpenNone
OpenNone
DuplicateNone
OpenNone
OpenNone
OpenNone
StalledNone
DuplicateNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
Resolvedppelberg
ResolvedKrinkle
OpenNone
OpenNone
OpenNone
ResolvedHalfak
OpenNone
OpenNone
StalledNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
ResolvedPetrb
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
Resolvedovasileva
Resolvedovasileva

Event Timeline

Mormegil created this task.Jan 27 2020, 4:16 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 27 2020, 4:16 PM

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:

desktop Diff:

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; }

Jdlrobson updated the task description. (Show Details)Jan 28 2020, 9:58 AM
matmarex updated the task description. (Show Details)Jan 29 2020, 6:50 PM
matmarex added a subscriber: matmarex.

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 claimed this task.Feb 3 2020, 4:28 AM
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

phuedx added a subscriber: phuedx.Feb 3 2020, 11:23 AM

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

Jdlrobson updated the task description. (Show Details)Feb 3 2020, 1:19 PM
Jdlrobson reassigned this task from Jdlrobson to Edtadros.Feb 4 2020, 9:16 AM
Edtadros reassigned this task from Edtadros to ovasileva.Feb 6 2020, 3:46 PM
Edtadros added a subscriber: Edtadros.

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

Edtadros updated the task description. (Show Details)Feb 6 2020, 3:46 PM
ovasileva closed this task as Resolved.Feb 13 2020, 11:17 AM

looks good, resolving.