Test the new wikidiff2 version on beta
Closed, ResolvedPublic

Description

The new version of wikidiff2 is on beta now and we all should test it there. Especially the changes in moved paragraphs functionality.

So grab a page, make some changes, move paragraphs and look at the diffs. :-)

http://en.wikipedia.beta.wmflabs.org/wiki/

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 19 2017, 2:38 PM

Pro-Tip:

  1. Look for a page to edit.
  2. Move & change paragraphs & text
  3. Click on Show changes to see wikidiff2 diff

Thanks! Anything specific you would like to see tested? Or is it more like having some tries to reveal glitches etc.?

@Jan_Dittrich in general just shoot at it with what ever you think makes sense.

Using the new diff view with a moved paragraph on RTL shows a problem with the indicator symbol and it's intended symbolic. On RTL languages the columns for the older and the newer version change places and the diff marker symbols ( and ) then do not point into the direction where the paragraph moves but away from it. See screenshot attached. In theory this could be fixed later with CSS but in the plain version this might be misleading.

Jan_Dittrich added a comment.EditedSep 22 2017, 1:34 PM

The problem seems to be that the arrows left- and right-pointy-ness is directly in the character. But what we actually mean is "point to the other column" (not right or left directly). I would suggest the following:

  • Put a character without a particular meaning (dot?) in an HTML element. Give the semantics ("point to other column") via CSS.
  • Via CSS we can set the "proper" arrow or icon or whatever we like.
  • For users who see the rendered HTML/CSS they get what was set by CSS
  • Users who use a screenreader could not make much use of the unicode character arrow anyway. They need a description anyway, so having a dot there should be no problem.

Thus, we could separate meaning and presentation and do this for rendered and screenreader representation. (@WMDE-Fisch)

When rolling out the new HHVM 3.18.5 bugfix release I reviewed the hhvm/error.log on mediawiki04.deployment-prep and noticed that the new wikidiff triggers various asserts in HHVM:

An examplary excerpt from the HHVM error.log is here: https://phabricator.wikimedia.org/P6052
The mentioned stack trace is here: https://phabricator.wikimedia.org/P6053

Hi @MoritzMuehlenhoff,

can you (or anyone) help interpret this output? I do x86 assembly, but I'm not familiar with php jit bytecode :)

Are you certain that the asserts are caused by our patch? i.e., is it possible that the same happens with old wikidiff2?

Have you noticed any other symptoms? Diffs show up OK, right?

Best,
Johannes

When rolling out the new HHVM 3.18.5 bugfix release I reviewed the hhvm/error.log on mediawiki04.deployment-prep and noticed that the new wikidiff triggers various asserts in HHVM:

An examplary excerpt from the HHVM error.log is here: https://phabricator.wikimedia.org/P6052
The mentioned stack trace is here: https://phabricator.wikimedia.org/P6053

I'm trying to reproduce it on my local machine. With my distro-supplied hhvm 3.22.0 (rel) I don't see anything suspicious in the logs at all so far. Can you pin down when the asserts happen? Probably when generating a mobile diff page?

It's certainly related to the new wikidiff2, these errors start being logged since Sep 15 13:24:31 which coincides with the time Adam pushed the config change to enable it: https://phabricator.wikimedia.org/T175818#3610346

I'm not sure if a proper diff is generated when those crashes occur, the rate has certainly increased (and probably correlated to increased testing):
Sep 16: 22
Sep 17: 44
Sep 18: 24
Sep 19: 62
Sep 20: 22
Sep 21: 40
Sep 22: 72
Sep 23: 88
Sep 24: 72
Sep 25: 118
Sep 26: 146
Sep 27: 152

If you try to reproduce it with HHVM 3.22 (which distro?), you need to rebuild the wikidiff package against the hhvm-dev package, did you do that? It's probably easier to setup a test environment based on the packages we use in production, e.g. with mw-vagrant. In addition you'll need to add the following apt source to pull in the new wikidiff:

deb http://apt.wikimedia.org/wikimedia jessie-wikimedia experimental 
deb-src http://apt.wikimedia.org/wikimedia jessie-wikimedia experimental

Recompiling wikidiff2 is not the issue here, I wrote the moved-paragraphs patch.

I'll see if I can reproduce it with mw-vagrant.

I think we have now fairly thoroughly tested this, especially with all of the investigation into T176637 which I think we can also close

Addshore closed this task as Resolved.
Addshore claimed this task.
Restricted Application added a project: User-Addshore. · View Herald TranscriptTue, Oct 10, 10:29 PM