Page MenuHomePhabricator

Zeroconf Parsoid/PHP seems to be considered as unconfigured by Flow Utils
Closed, ResolvedPublic

Description

Flow\Conversion\Utils::isParsoidConfigured() checks if Parsoid is configured by checking $wgVirtualRestConfig(and deprecated $wgFlowParsoidURL things).

In the consequence, a zero-configuration Parsoid is ignored partially by the editor and the editor seems to be partially broken. 'Partially' means, from HTML to wikitext converting button is enabled but the output is an error message.

I could not test strictly because I am not familiar with Flow and/or Parsoid enough. Is it right or I did something wrong?

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 622258 had a related patch set uploaded (by Lens0021; owner: Lens0021):
[mediawiki/extensions/Flow@master] Do not throw exception for zero-conf Parsoid

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

Change 622258 abandoned by Lens0021:
[mediawiki/extensions/Flow@master] Do not throw exception for zero-conf Parsoid

Reason:

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

Change 622258 restored by C. Scott Ananian:
[mediawiki/extensions/Flow@master] Do not throw exception for zero-conf Parsoid

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

Change 622579 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/extensions/ContentTranslation@master] Do not throw exception for zero-config VisualEditor

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

Change 622579 merged by jenkins-bot:
[mediawiki/extensions/ContentTranslation@master] Do not throw exception for zero-config VisualEditor

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

To avoid horrible messages for users of MW 1.35 (see T267407), I temporarily applied the following fix (extracted from the patch currently in preparation) and the round-trip conversion works fine now:

sebastien@fermewiki3 15:00:31 /opt/mediawiki/1.35.0/extensions/Flow $ git diff
diff --git a/includes/Conversion/Utils.php b/includes/Conversion/Utils.php
index 711b25587..3b7981c38 100644
--- a/includes/Conversion/Utils.php
+++ b/includes/Conversion/Utils.php
@@ -295,7 +295,7 @@ abstract class Utils {
        private static function makeVRSObject() {
                global $wgVirtualRestConfig, $wgFlowParsoidURL, $wgFlowParsoidPrefix,
                        $wgFlowParsoidTimeout, $wgFlowParsoidForwardCookies,
-                       $wgFlowParsoidHTTPProxy;
+                       $wgFlowParsoidHTTPProxy, $wgVisualEditorParsoidAutoConfig;
 
                // the params array to create the service object with
                $params = [];
@@ -315,6 +315,12 @@ abstract class Utils {
                        // there's a global parsoid config, use it next
                        $params = $vrs['modules']['parsoid'];
                        $params['restbaseCompat'] = true;
+               } elseif ( $wgVisualEditorParsoidAutoConfig ) {
+                       $params = $vrs['modules']['parsoid'] ?? [];
+                       $params['restbaseCompat'] = true;
+                       // forward cookies on private wikis
+                       $params['forwardCookies'] = !MediaWikiServices::getInstance()
+                               ->getPermissionManager()->isEveryoneAllowed( 'read' );
                } else {
                        // no global modules defined, fall back to old defaults
                        if ( !$wgFlowParsoidURL ) {

Thank you Seb35, it works! When will the official patch be released?

The patch is also working for me on MW 1.36.1 and Flow from branch REL1_36. Is there any plan to get it merged?

Anyone is very welcome to amend the patch to make it pass jenkins-bot

Moved this to Not a blocker because actually the previous release, 1.36, was not blocked.

Thank you, Seb35. Also applies to the REL1_37 branch. I hope this patch will be reflected in the master branch as well.

Tinss changed the task status from Open to In Progress.Jan 19 2022, 11:36 PM
Tinss subscribed.

The patch in https://gerrit.wikimedia.org/r/622579 has been merged, it should make it master. Can that task be marked resolved?

Aklapper changed the task status from In Progress to Open.Feb 5 2022, 7:46 AM

@Tinss: I'm afraid https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Flow/+/622258 still needs rework and a merge before resolinv this ticket.

The REL1_35, REL1_36 and REL1_36 branches also need to reflect the modifications.

@Aklapper I would like to get this merged. All test results on the ticket seem to be gone, so I do not know what work should be done. What needs to be changed to the patch?

@MeneerWout: Going to https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Flow/+/622258 and selecting "Rebase", I get "Could not perform action: The change could not be rebased due to a conflict during merge. merge conflict(s): includes/Conversion/Utils.php" so this needs manual rebasing and amending, I think

matmarex edited projects, added MW-1.41-notes; removed Patch-Needs-Improvement.
matmarex subscribed.

This problem should no longer occur on MediaWiki 1.41 and later. In T337223 the Parsoid integration has been redone, and it should be much more reliable now.

Change 622258 abandoned by Bartosz Dziewoński:

[mediawiki/extensions/Flow@master] Do not throw exception for zero-config VisualEditor

Reason:

Patch no longer applies, this code has been removed in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Flow/+/924478

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