Page MenuHomePhabricator

PageForms display=spreadsheet fieldset error when no template label
Closed, ResolvedPublic

Description

I have a large form that I am splitting it up with HeaderTabs for readability.

I am trying to utilize the display=spreadsheet for a few of the multiple template form fields, and found a bug when there is a lack of label tag on the for template call.

Steps to reproduce:
When {{{for template|template_name|multiple|label=Label|display=spreadsheet|embed in field=main[tn]}}} is in the form definition, the form entry page looks normal:

image.png (358×374 px, 4 KB)

When {{{for template|template_name|multiple|display=spreadsheet|embed in field=main[tn]}}} is in the form definition, the form entry page looks messed up:
image.png (328×514 px, 11 KB)

My Temporary Fix
I don't know php very well, but I read through and found a spot where the call to the display=spreadsheet in PF_FormPrinter.php was adding a </fieldset> tag regardless if the <fieldset> tag was being called earlier in the FormPrinter. I did a quick edit to see if I could fix it... and it seems I could. I'm not sure if it is the best solution though for this bug due to my lack of experience.

diff --git a/includes/PF_FormPrinter.php b/includes/PF_FormPrinter.php
index 88d110fd..3d019950 100644
--- a/includes/PF_FormPrinter.php
+++ b/includes/PF_FormPrinter.php
@@ -1595,7 +1595,9 @@ END;
                                                $multipleTemplateHTML .= $this->spreadsheetHTML( $tif );
                                                // For spreadsheets, this needs
                                                // to be specially inserted.
-                                               $multipleTemplateHTML .= "</fieldset>\n";
+                                               if ( $tif->getLabel() != null ) {
+                                                       $multipleTemplateHTML .= "</fieldset>\n";
+                                               }
                                        }
                                } elseif ( $tif->getDisplay() == 'calendar' ) {
                                        if ( $tif->allInstancesPrinted() ) {

Event Timeline

Change 655395 had a related patch set uploaded (by Yaron Koren; owner: Yaron Koren):
[mediawiki/extensions/PageForms@master] Fix for "display=spreadsheet" with no template label

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

Change 655395 merged by jenkins-bot:
[mediawiki/extensions/PageForms@master] Fix for "display=spreadsheet" with no template label

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

Yaron_Koren claimed this task.
Yaron_Koren subscribed.

@RinaCY - thanks for the patch! It works perfectly. I just checked it in.