Page MenuHomePhabricator

"hidden", "mapping property" break multiple-instance templates in SF 3.4.2
Closed, ResolvedPublic

Description

Hey

I recently upgraded to SF3.4.2 (f8d3908). Now it seems the multiple-instance template feature is broken:

  • on the form page, adding another instance has all defaults ignored
  • on form save all but the first instance's data is lost. only the first instance is processed correctly

I also tried SF "3.4.3-alpha (e8fbb917f128d6c29a81214a6dddf1d3404b5024) 27. Jan. 2016, 18:53". It still looks broken. Also with this version, textarea fields and free text has the wikieditor missing (apparently due to a javascript bug. console is mumbling something about "initFunction not being a function". Sorry, can't be of much help there).

PS: everything works fine with 3.4

My Setup:

  • MW 25.5
  • SMW 2.3.1
  • SF 3.4.2 - 3.4.3-alpha
  • SFI 0.7 up to 0.10

Best wishes!

Event Timeline

Oetterer raised the priority of this task from to High.
Oetterer updated the task description. (Show Details)
Oetterer added a subscriber: Oetterer.
Oetterer renamed this task from Multiple Templates seem to be broken with 3.4.2 to Multiple-instance templates seem to be broken with 3.4.2.Feb 12 2016, 5:14 PM
Oetterer updated the task description. (Show Details)
Oetterer set Security to None.

If you deactivate SFI, does this JS error still occur?

Just tested with deactivated SFI. Still there:

TypeError: initFunction is not a function
https://<host.domain>/w/extensions/SemanticForms/libs/SemanticForms.js
Line 307

Change 270334 had a related patch set uploaded (by Foxtrott):
Fix calling init functions of input types

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

Change 270334 merged by Foxtrott:
Fix calling init functions of input types

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

The "initFunction not being a function" should be fixed now. With any luck the rest as well. Could you pull the lastest SF and try?

can confirm. wikieditor is back online! thank you for fast response and the good work! :)

unfortunately the issues with multiple-instance templates persist.

@Oetterer please could you try the latest code from master branch.

Just did an update for the master. Seems to me, there are only Localisation updates...

git pull
[..]
From https://git.wikimedia.org/git/mediawiki/extensions/SemanticForms
   72c7267..4131540  master     -> origin/master
Updating 72c7267..4131540
Fast-forward
 i18n/ca.json | 1 +
 i18n/vi.json | 5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

Does any off the links I linked look like they caused the problem.

Could you link to, or pastebin, or include here your form definition?

You can find two examples here. I only included the <includeonly> part. And not just for the pun. Unfortunately, all the user interface text is german.

Some things I noticed while compiling them:

  • the problem with the dropped default values and the ignored multi-instance templates does only occur on edit, not on create
  • creating:
    • when creating entities with a multi-instance template, all defaults are there on every instance
    • I can create as many instances as I like, all are processed and parsed correctly
  • editing
    • when I edit them, only the already created instances are parsed and processed. new ones are dropped/ignored
    • default values for the multi-instances are ignored on newly added instances
    • deleting instances (with the big X on the right) works for all except the first one. the first one I can delete on the form but it is still there after save

PS: Just did a pull on master (4131540) before these tests

One more thing: #autoedit on a field on the main template erases all the multi instance templates. :(

@Yaron_Koren do you know how to fix this problem.

This is interesting... apparently hidden inputs, and the "mapping property" parameter, both cause problems with saving when used within multiple-instance templates. Those appear to be the issues with the "Extension" and "Chapter" forms, respectively. I'm still investigating this, but that's what I know at the moment.

Yaron_Koren renamed this task from Multiple-instance templates seem to be broken with 3.4.2 to "hidden", "mapping property" break multiple-instance templates in SF 3.4.2.Feb 17 2016, 6:56 PM
Yaron_Koren lowered the priority of this task from High to Medium.

@Oetterer - okay, I just checked in a change that I believe fixes all these issues. If you can, please try it out and let me know!

I ran some tests:

  • working: recognition of defaults. on create and edit
  • working: creating entities with multi-instance templates
  • working: adding instances on edit
  • working: removing instances on edit except
  • not working: removing the last instance (see below)
  • not working: #autoedit still removes all the multi-instance templates when I called it to edit the main template

Everytime I tried to remove all instances - whether it be one or two - as soon as I saved the form with none of the multi-instances in the form the page was saved but the template calls were not removed. So: removing works as long as I don't try to remove the last instance. Then all deletions are ignored and the page saves unchanged.

Let me know if I can be of more assistance.

Hi did you updated to master branch again since Yaron may have fixed the problem for you.

I did test on fd24011d98840c65d8a19b8778b159d0e8b4f00c. git pull claims master is up-to-date.

Ok then I think the problem is still happening @Yaron_Koren

Oh, interesting. I see this problem too, but only when all instances of the template are removed (and again, only when there's a "hidden" or "mapping" parameter). There doesn't seem to be anything significant about the last instance; or am I missing something?

No, you're right. They should all be the same.

@Oetterer could i ask you something, If it worked in 3.4.0 then could you from the point that, that release was branch could you try every patch after the 3.4.0 was released that way we know when the problem was introduced and what patch. If it was https://github.com/wikimedia/mediawiki-extensions-SemanticForms/commit/8a9048b540378987e9934c326933837d1df7d1f8 then i think it may be because i moved the functions up and down in the files because of jshint 2.9.1 was complaning off it, to resolve it we may need to move the functions back to were they were but also update the jshint config so that error doesent happend again. But with https://github.com/wikimedia/mediawiki-extensions-SemanticForms/commit/705c4dc1030647ee0eee12ade7a54009e520e9ee it may be tricky to revert but we should instead find where the fault it only if these patchs are causing the error.

Sorry to ask you do that but we may be able to resolve the issue quickly.

but also since you said it only worked in 3.4 but did not work in 3.4.1 then you could compare from

https://github.com/wikimedia/mediawiki-extensions-SemanticForms/commits/master?page=7

too

https://github.com/wikimedia/mediawiki-extensions-SemanticForms/commits/master?page=5

Did 3.4.1 work for you.

@Paladox - there have been a ton of changes since 3.4; I don't think it's worth the effort trying to hunt down the specific change that caused this error.

Yes I was looking through the changes, I manly looked between 3.4 and 3.4.1 and may have only found one patch that changed what this patch has title. It is to do with something about tree and to do with mutiple template instance and is in SemanticForms.js.

Maybe not look though changes since there is a ton. But what about between 3.4 and 3.4.1.

yeah. tedious work. I got the implression, we are dealing with multiple sources.

Also: I have an addendum to my comment from Fri, Feb 19, 5:42 AM
on master:
#autoedit works with Form:Extension
#autoedit works NOT with Form:Class (see http://homepages.uni-paderborn.de/oetterer/FormExamples/class.html), resulting - as stated - in the removal of all instances.

now the tests I ran so far, all but 5 with Form:Extension (http://homepages.uni-paderborn.de/oetterer/FormExamples/extension.html)
0 recognition of defaults
1 on manual edit: adding element in multiinstance
2 on manual edit: removing one element in multiinstance
3 on manual edit: removing all instances
4 autoedit with form:extension
5 autoedit with form:class

                Tests
commit  | 0  | 1  | 2  | 3  | 4  | 5
--------------------------------------
a9a0b56 | Y  | Y  | Y  | Y  | Y  |N[0]
--------------------------------------
fc91b44 | N  |N[3]|N[3]|N[3]|N[3]|N[4]
--------------------------------------
f8d3908 | N  | N  | Y  | N  |N[1]|N[2]
--------------------------------------
43576f4 | ?  | N  | Y  | N  | Y  | Y 
--------------------------------------
fd24011 | Y  | Y  | Y  | N  | Y  | N

a9a0b56 is 3.4.1
f8d3908 is 3.4.2
fd24011 is currently master

I will stop for now - a man's got to eat :) Also: going to watch Deadpool! :)
If you need me to go on running tests, just say so. for now, I am on hold. Best of luck!

[0]: strange bug where
"|parameter_parameter={{Classgenerator/parameter" got replaced with "|parameter_parameter=}}{{Classgenerator/parameter"

[1]:
expected:

{{Extension
|description=Test_4
|documentation_link=https;//www.mediawiki.org/wiki/Extension:Test2
|include_command=require_once("$IP/extensions/Test2/Test2.php");
|is_default=Nein
|configuration={{Configuration setting
|name=$wgTest1
|type=Boolean
|default_value_boolean=true
|is_unset=Nein
|description=asd
|parent=Extension:Test2
}}{{Configuration setting
|name=$wgTest2
|type=Boolean
|default_value_boolean=true
|is_unset=Nein
|description=asd
|parent=Extension:Test2
}}
}}

produced:

{{Extension
|description=Test_4
|description=Test_3
|documentation_link=https;//www.mediawiki.org/wiki/Extension:Test2
|include_command=require_once("$IP/extensions/Test2/Test2.php");
|is_default=Nein
|configuration={{Configuration setting
|name=$wgTest1
|type=Boolean
|default_value_boolean=true
|is_unset=Nein
|description=asd
|parent=Extension:Test2
}}{{Configuration setting
|name=$wgTest2
|type=Boolean
|default_value_boolean=true
|is_unset=Nein
|description=asd
|parent=Extension:Test2
}}
|description=Test_3
|documentation_link=https;//www.mediawiki.org/wiki/Extension:Test2
|include_command=require_once("$IP/extensions/Test2/Test2.php");
|is_default=Nein
|configuration={{Configuration setting
|name=$wgTest1
|type=Boolean
|default_value_boolean=true
|is_unset=Nein
|description=asd
|parent=Extension:Test2
}}{{Configuration setting
|name=$wgTest2
|type=Boolean
|default_value_boolean=true
|is_unset=Nein
|description=asd
|parent=Extension:Test2
}}
}}

[2] similar as in [1]
[3] after edit, all data was lost, resulting in

{{Extension}}
{{Configuration setting}}
{{Configuration setting}}
{{Configuration setting}}

[4] similar to [3]

I don't think these tests are useful, actually... sorry you did all that work. I'm looking into the issue now, though.

Okay, I think the instance-removal issue is fixed now - it turned out to be a problem specifically with embedded multiple-instance templates. I just checked in a fix; please let me know if that works for you.

I haven't looked into the autoedit thing.

Hey.

Sorry, I did not find the time to do the tests yesterday. Just updated (to 3a53480) and ran some tests:

I know, you didn't look into the #autoedit thing, but nevertheless I thought, I'd check. :)

Thanks for the great work so far!

Great! I think I just checked in a fix for the autoedit bug as well. I'm marking this as resolved; if that latest bug fix doesn't work, please create a new ticket, because it's a different issue.

All works well, thank you very much. Awesome work. You're the best! :)