But why is $imgContHorizontal undefined here?

@Samwilson @Soda You may want to be aware of this issue.

How big is the impact? Should this be a train blocker?

Ah, oh, yes, I see. When we start, $imgContHorizontal is initialized to null (what's the point of that long chain of vars getting set to null at the top? They should either be set to something useful or their initialization might as well be deferred until first used.). It doesn't actually get assigned any value until toggleLayout() is called, and there it's wrapped inside a conditional such that it is only initialized if the layout is already horizontal (if (!isLayoutHorizontal)). When we start in vertical mode the var is never initialized, until something actually triggers toggleLayout(); but then a few lines later $imgContHorizontal.add() is called regardless of which branch was picked in the preceding conditional.

The correct fix is probably for that if to be rewritten so it doesn't make assumptions about starting and ending state: tear down the layout we're leaving and set up the one we're entering, every time. $imgContVertical gets inited in initEnvironment() but not $imgContHorizontal; and vice versa in toggleLayout(). And the code in both place is actually the same apart from the variable name and CSS class name, so this is probably a tiny little init function even if not collected to one place otherwise.

Change 751946 had a related patch set uploaded (by Tpt; author: Tpt):

[mediawiki/extensions/ProofreadPage@master] Makes sure $imgContHorizontal is always initialized

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

In T298694#7601710, @Zabe wrote:

How big is the impact? Should this be a train blocker?

My assessment—but I am probably not the best to assess that—is that it's not a train blocker. It completely breaks page zoom and pan on all Wikisourcen, and may just possibly be causing some other UI problems there, but it shouldn't affect any non-Wikisource projects and it doesn't break primary workflow in most cases. It's going to be plenty annoying, and getting a fix out fast is a good idea, but neither rolling back nor holding the current train seems merited.

I wrote a change that should fix the bug https://gerrit.wikimedia.org/r/c/mediawiki/extensions/ProofreadPage/+/751946/

@Inductiveload @Zabe @Xover Would it be possible that one of you has a look at it? it would be great to push it in one of today backport windows.

Sorry, I can't test it today, I don't have my usual setup nearby!

OK, I lied, I can do a patch demo: https://patchdemo.wmflabs.org/wikis/06ce750a61/wiki/Page:Shen_of_the_Sea.pdf/45

Test wiki created on Patch demo by Inductiveload using patch(es) linked to this task:

https://patchdemo.wmflabs.org/wikis/06ce750a61/w/

Patchdemo looks good to me: no console errors, all the OSD stuff (zoom, rotate, etc.) looks like it works, and it even seems to fix the unresponsive radio buttons that were reported on-wiki.

The only thing I notice is that the edit form is a bit short / not tall enough. That might be just the environment on Patchdemo, or it might be a side-effect of the actual patch (for example because we now always initialize horizontal layout and then always get the sizing from that, instead of from vertical layout). If it's a side-effect we'll get howls from users until that too is fixed, but it's still preferable, IMO, to the current completely broken state for the OSD stuff.

Looking at the code in the patch it looks good to me, but, hey, there's a reason I don't have +1 on anything around here.
:-)

@Inductiveload Thank you!

@Xover Thank you! The height problem seems an other consequence of c6f71a5399ebb2718aa86b2842d52e7bd207ed49: the edit box height is not anymore set from the image size. I have updated my PS to fix the problem.

Test wiki created on Patch demo by Xover using patch(es) linked to this task:

https://patchdemo.wmflabs.org/wikis/5dcbb0d0f7/w/

Updated patch looks good to me, but I'm not sufficiently familiar with the context there for that to mean much.

But I muddled my way through setting up an updated patchdemo for the revised patch (against master since the 1.38 branch looks broken right now) and so far as I can tell it works correctly.

So as far as I'm concerned this is ready to go out.

Change 751843 had a related patch set uploaded (by Tpt; author: Tpt):

[mediawiki/extensions/ProofreadPage@wmf/1.38.0-wmf.16] Makes sure $imgContHorizontal is always initialized

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

Change 751946 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Makes sure $imgContHorizontal is always initialized

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

Test wiki on Patch demo by Inductiveload using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/06ce750a61/w/

Crap, I notice we don't have any regular backport windows until Monday. While this wasn't train-blocking/rollback when noticed, it's probably UBL-y enough that it needs to go out as an emergency backport. It doesn't break the primary workflow in that the Wikisourcen can still edit pages, it does break the zoom/pan for the reference page image (which is highly-used functionality) and partially breaks (blocks) the UI for setting page status (particularly for people with page zoom on in the browser, which is a major accessibility issue and primary workflow).

Any chance we could get it pushed today? I'm available for testing until a little before midnight Central European.

@Xover Sadly there is not hope for it to be pushed today except if someone is managing to follow the emergency deployment process: https://wikitech.wikimedia.org/wiki/Deployments/Emergencies

If some of you could join #wikimedia-operations over the day, an emergency deployment can probably be organised.

I have approved the change and we can deploy it as soon as it has merged

Change 751843 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@wmf/1.38.0-wmf.16] Makes sure $imgContHorizontal is always initialized

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

Mentioned in SAL (#wikimedia-operations) [2022-01-07T12:14:37Z] <taavi@deploy1002> Synchronized php-1.38.0-wmf.16/extensions/ProofreadPage/modules/page: Backport: [[gerrit:751843|Makes sure $imgContHorizontal is always initialized (T298694)]] (duration: 00m 59s)

The user that first reported this issue on enWS has tested and confirmed that it is now fixed for them. And my own testing concurs.

Test wiki on Patch demo by Xover using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/5dcbb0d0f7/w/