Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JavaScript error in Form Builder when opening section #4011

Closed
avernet opened this issue Apr 3, 2019 · 14 comments

Comments

2 participants
@avernet
Copy link
Collaborator

commented Apr 3, 2019

To reproduce, use the form and follow the steps in this message.

@avernet avernet self-assigned this Apr 3, 2019

@avernet avernet added this to To do in Orbeon Forms 2018.2.3 via automation Apr 3, 2019

@avernet

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 3, 2019

The issue happens when running:

<xxf:copy-repeat-template
    id="fb≡research-section≡xf-4638≡c-equipment-section≡c-equipment-nested-section≡grid-7-grid≡grid-7-grid-repeat" 
    parent-indexes="1" start-suffix="1" end-suffix="1"/>

The c-equipment-nested-section is the section we're trying to open, and it contains a repeated grid. The section was collapsed because the form has more than 100 sections (the default threshold per oxf.fb.section.close). So this seems to be related to xxf:update="full" with repeated content.

A workaround is to increase the value for oxf.fb.section.close.

@avernet

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 3, 2019

I attached a version the form source with more descriptive ids, and the Ajax response that is triggering this problem to this message.

@avernet avernet assigned ebruchez and unassigned avernet Apr 3, 2019

@ebruchez ebruchez added this to To review in Orbeon Forms 2019.1 via automation Apr 4, 2019

@avernet avernet added the Crasher label Apr 10, 2019

@ebruchez

This comment has been minimized.

Copy link
Collaborator

commented Apr 18, 2019

The nested section that opens contains a repeat. The markup is there even when the section is closed, but there are initially no iterations:

<div id="repeat-begin-fb≡research-section≡xf-193≡c-equipment-section≡c-equipment-nested-section≡grid-7-grid≡grid-7-grid-repeat⊙1"
     class="xforms-repeat-begin-end"></div>
<div id="repeat-end-fb≡research-section≡xf-193≡c-equipment-section≡c-equipment-nested-section≡grid-7-grid≡grid-7-grid-repeat⊙1"
     class="xforms-repeat-begin-end"></div>

When <xxf:copy-repeat-template> is interpreted, a new iteration is to be added, but the template is nowhere to be seen. So the question is: where should the template be, and why is it not output in this case?

@ebruchez

This comment has been minimized.

Copy link
Collaborator

commented Apr 18, 2019

Our general policy to output repeat templates is:

  • isTopLevelRepeat == true
  • or isTemplate == true, which means that we are generating a template for a nested repeat

In short, repeat templates are produced for a top-level template and includes nested repeat templates.

Here, we have a situation with an initially-closed top-level section. This means that, initially, nested sections do not output their markup. In fact, their nested control trees are empty. This also means that the nested repeat templates are not created.

So we need a solution for this. Clearly, since there is no control tree output when the outer section is closed, we need to output the repeat templates later. That "later" would have to be when the nested markup is output for the first time.

@ebruchez

This comment has been minimized.

Copy link
Collaborator

commented Apr 22, 2019

It's not only that "the nested repeat templates are not created", but that the top-level repeat templates can be incomplete.

So even an initially-closed top-level repeated section that doesn't contain a nested repeat can fail if it contains a nested section as the template for that one will not be present.

See this gist for example.

@ebruchez

This comment has been minimized.

Copy link
Collaborator

commented Apr 23, 2019

I don't think there is a way besides adjusting (re-generating) the template.

Should the repeat template even be generated in the first place, since it is not usable? How can we know? We would need to search the control tree under the repeat to see if there is any component with bindingOpt.isEmpty. If so, skip the repeat template. That appears possible. But then, this would mean to produce the template at a later time, and tell the client to store that template.

There is also be the case where a lazy binding component is nested further. So it might be that a repeat template is needed and updated, and then further updated later if needed.

Could a way out be to never produce repeat templates on the client and always send them from the server as needed? Is there a smaller change possible?

Right now, we have:

  • templates produce upon the initial page load
  • a possibly large sequence of instructions to setup things in the template

It might just make sense to no longer do that and simply do a "full update" for new repeat iterations.

In a first step, we could do this only in Form Builder if we want to be conservative.

@ebruchez

This comment has been minimized.

Copy link
Collaborator

commented Apr 24, 2019

  • initial implementation
  • fix "XBL full update within full update is not supported"
  • error in complex form when adding iteration before (#4035)
  • enable full updates only for nested parts
  • update tests
  • remove support for templates for 2019.1
    • xforms.js
    • ControlsComparator and related (schema)
    • XFormsRepeatHandler must not output templates
    • update tests
@ebruchez

This comment has been minimized.

Copy link
Collaborator

commented Apr 24, 2019

A Mark is an object, stored in the static part Metadata, which points to a specific point of the form's HTML/XForms template. Using a Mark (metadata.hasTopLevelMarks) will cause the entire template to be preserved as part of the static state. If a form has no Mark, the template will not be preserved (or at least, should not).

So far, Marks are only used when a control is explicitly marked with xxf:update="full". This is the case for:

  • forms using the wizard (<xf:switch xxf:update="full">
  • Form Builder (annotate-design-time.xsl)
  • Summary and Home pages

So that cover many common cases. If we change the handling of repeats in general, we would keep the template for a larger number of forms. I don't think that is a problem, just something to be aware of.

@ebruchez

This comment has been minimized.

Copy link
Collaborator

commented Apr 24, 2019

The above only applies to top-level usages of repeats / full updates. Usages within XBL components require the template of the XBL component, which is preserved in the static state, as the content of the XBL bound node is preserved. So it seems that we are keeping even more stuff. See #2128.

If anything, this tells us that it is ok to keep stuff for repeats.

@ebruchez

This comment has been minimized.

Copy link
Collaborator

commented Apr 25, 2019

It'd be nice to remove all the code that handles the isTemplate mode in the handlers. This causes extra work in the handlers to add the ability to output markup even when there is no concrete control.

Removing repeat template generation is an easy first step. But there is still the scenario where there is no repeat iteration, and we want to figure out what name the begin/end delimiter must have. For example, iterating around a <xh:th> should output that, while iterating around an <xf:output> should output an <xh:span>, etc. That's a lot of work to do just for making that call!

@ebruchez

This comment has been minimized.

Copy link
Collaborator

commented Apr 25, 2019

The good news is that, when instantiating a handler, we can call its getContainingElementName without doing all the processing for the control. So if we can find the handler, we can find the element name, and we don't need to go through all that complex code producing a dummy template.

ebruchez added a commit that referenced this issue Apr 26, 2019

@ebruchez

This comment has been minimized.

Copy link
Collaborator

commented Apr 26, 2019

After refactoring, it should now be fairly easy to use the getContainingElementName way.

@ebruchez

This comment has been minimized.

Copy link
Collaborator

commented May 1, 2019

Closing. The fix is backported to 2018.2, and in addition, the code is refactored to remove support for templates for 2019.1.

@ebruchez ebruchez closed this May 1, 2019

Orbeon Forms 2019.1 automation moved this from To review to Done May 1, 2019

Orbeon Forms 2018.1.4 automation moved this from Todo to Done May 1, 2019

Orbeon Forms 2018.2.3 automation moved this from To do to Done May 1, 2019

@ebruchez ebruchez moved this from Done to Todo in Orbeon Forms 2018.1.4 May 1, 2019

@ebruchez

This comment has been minimized.

Copy link
Collaborator

commented May 9, 2019

More fixes were required and tagged with #4011.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.