1389969 - False positive trailing and leading space warnings in diff

Status: RESOLVED FIXED

(Webtools Graveyard :: Pontoon, defect, P2)

Description

[Théo Chevalier]

Attachment: Screenshot

In the DIFF view, I can see leading and trailing space warnings in the middle of strings, but they seem false positives.

I’ve attached an example from here https://pontoon.mozilla.org/fr/copyright-campaign/copyright.properties/?string=169864

Comment 1

[Matjaz Horvat [:mathjazz]]

Possibly related: in string that contain newlines, newlines are removed in the diff view.

See:
https://pontoon.mozilla.org/sl/firefox/all-resources/?search=%3Cul&string=78875

Comment 2

[Jarek Śmiejczak [:jotes]]

Currently `diff()` function runs `markPlaceables` at every chunk of text returned by `diff.js`.
Because of that, function marks spaces between words as leading/trailing.

Comment 3

[Jarek Śmiejczak [:jotes]]

:mathjazz
About newlines:

I've set `white-space: pre-wrap` for `.translation-diff` class, locally and it seems to fix display of newlines in the diff.
Could you check if that's a reasonable approach?

Comment 4

[Matjaz Horvat [:mathjazz]]

Yup, that solves this part of the problem.

Comment 5

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/pontoon/pull/836

Comment 6

[[github robot]]

Commit pushed to master at https://github.com/mozilla/pontoon

https://github.com/mozilla/pontoon/commit/a48bc0b9c40cd46737761ec77e968ecfd41ba0c1
Fix bug 1389969 - Fix display of trailing/leading spaces in the diff view. (#836)

Major changes:
* markPlaceables() is applied only once in diff(), that allows to mark
  leading/trailing spaces correctly
* fixed issue with missing trailing/leading spaces for multiline strings

Minor changes:
* use a switch statement instead of three if blocks
* use constants provided by Diff.js instead of magic numbers