Page MenuHomePhabricator

Unwanted linebreaks in diffs when span.diffchange is present
Closed, ResolvedPublic

Description

When adding some CSS to my userskin in order to show indents in diffs (see url for code), it revealed that DifferenceEngine.php (I think) is inserting unwnated linebreaks in the output HTML. While not 'illegal', it does thwart the intended effect. This only happens if the line contains a span.diffchange (the red text); other lines are fine. Below is the HTML output; note that line without a diffchange are one line, but the lines with a diffchange have a linebreak after <div> and before </div>, which breaks pre-wrap behaviour. Screenshot: http://en.wikipedia.org/wiki/File:Diff_with_unwanted_linebreaks.gif

<tr>

<td class="diff-marker"> </td> 
<td class="diff-context"><div>}</div></td> 
<td class="diff-marker"> </td> 
<td class="diff-context"><div>}</div></td>

</tr>
<tr>

<td class="diff-marker"> </td> 
<td class="diff-context"><div>/* Preserve white-space in diffs */</div></td> 
<td class="diff-marker"> </td> 
<td class="diff-context"><div>/* Preserve white-space in diffs */</div></td>

</tr>
<tr>

<td class="diff-marker">-</td> 
<td class="diff-deletedline"><div> <!--unwanted linebreak-->

table.<span class="diffchange">div </span>td div {<!--unwanted linebreak-->

</div></td> 
<td class="diff-marker">+</td> 
<td class="diff-addedline"><div> <!-- unwanted linebreak -->

table.<span class="diffchange">diff </span>td div {<!--unwanted linebreak-->

</div></td>

</tr>
<tr>

<td class="diff-marker"> </td> 
<td class="diff-context"><div>  white-space: pre-wrap;</div></td> 
<td class="diff-marker"> </td> 
<td class="diff-context"><div>  white-space: pre-wrap;</div></td>

</tr>
<tr>

<td class="diff-marker"> </td> 
<td class="diff-context"><div>}</div></td> 
<td class="diff-marker"> </td> 
<td class="diff-context"><div>}</div></td>

</tr>


Version: unspecified
Severity: minor
URL: http://en.wikipedia.org/w/index.php?title=User:Edokter/vector.css&diff=prev&oldid=393815275

Details

Reference
bz25725

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:19 PM
bzimport added a project: wikidiff2.
bzimport set Reference to bz25725.

Created attachment 7769
Screenshot showing fault

Attached:

Diff_with_unwanted_linebreaks.gif (179×842 px, 6 KB)

Theoretically, shouldn't the diff engine be using non-breaking spaces for any significant whitespace so using css white-space type hacks are unnecessary?

That would be an idea. On the other hand, if the original text doesn't contain non-breaking spaces, neither should the diff text, as it would introduce unwanted characters during copy/paste.

It seems the linebreaks are used as word boundaries, and the the diff line routines are reused for word level comparison.

Fixed in r75769, by line-ending squashing the work comparison results.

The report is against Wikipedia. r75769 doesn't fix it there since it patches the PHP diff algorithm, not wikidiff2. You can tell that the quoted HTML comes from wikidiff2 because of the pretty-printing. Reopening.

It turns out there was no problem with the PHP version. Reverted r75769 and fixed the actual problem in r80621. Still needs packaging and deployment.