CCALI / a2jviewer

This is the repo for the A2J Viewer
https://www.a2jauthor.org
Other
5 stars 8 forks source link

A2J Viewer failing to save answers and acting buggy in Firefox #244

Closed JessicaFrank closed 5 months ago

JessicaFrank commented 1 year ago

I don't think we have a singular issue for the Firefox Viewer problems, so creating one here. If there is one elsewhere, please tag it and I'll merge the issues.

Reports of answers not being saved in GIs run in Firefox after Dec 2022 Firefox browser update. Initial reports from NY GIs related to TF variables. However, debugging to isolate the problem seems to show initial errors occur much earlier in the interview and manifest later on in the NY one.

I created a couple test GIs to try to isolate the initial corruption point. In this interview, I added several lorem ipsum screens to mimic the NY GI experience where there are several intro screens with just continue buttons. Then I had the GI go to the name page. Before the name page, the GI acts as expected and var list appears normal in dev console. After hitting the name page, the var list disappears. In different tests it disappeared after 2nd field had data. In later tests (see below), it disappeared after entering a value for first field.

image

image

No errors logged in the console.

Exiting preview and then going back into preview seems to "reset" the vars list and it appears normal. It was fine with entering middle initial in 2nd field, then again disappeared after entering a value into 3rd field.

Exiting and re-entering preview a couple times shows a very slow interview with lags in moving to other questions. Also blank screen when I go directly into Preview from the navigation tabs. I am able to enter preview mode from the question design editor though.

GI used for testing. Firefox test Feb 13 2023(1).zip

GIF of odd behavior during testing firefox test 1

tobiasnteireho commented 1 year ago

probably related to #241

JessicaFrank commented 1 year ago

May be related to this: https://github.com/CCALI/CAJA/issues/1463

Daisy chain of before logic only pages with no content that bounce users around the GI. In testing, I hit 5 before my interview "broke".

Will update testing notes here tomorrow.

JessicaFrank commented 1 year ago

Theory on what's happening based on heavy testing yesterday in LHI's RebuildQA (which has 8.1.4 version of the viewer from July 2022)--

The daisy chain of logic in NY interviews has been an issue in the past (See ). I think it was a timing issue the last time we saw it -- the logic was skipping too fast over pages and the viewer was getting confused as to what page was supposed to be displayed when.

Here, I think the skipping due to logic is causing the A2J Viewer to think there isn't a variable in a field when there actually is a variable. It's resulting in the viewer inserting [Unassigned Variable 1] and storing the user's answers to subsequent fields in that variable (sometimes over and over again). [Unassigned Variable 1] was a fail safe we built in a couple years ago because authors would mistakenly leave a field without a variable assigned to it, a user would answer it, then the value would get stored in a variable called something like \undefined and HotDocs would break on assembly. [Unassigned Variable 1] is only supposed to be assigned to a field when the field doesn't have a variable at the point in which the user sees it (not automatically assigned to a blank field, but only assigned when a blank field is shown to the user in the viewer).

So I think we need to look into the sequence of when [Unassigned Variable 1] fires. Something about Firefox updating in December 2022 changed how the viewer handles daisy chain logic and causes a race condition where the A2J Viewer thinks the field doesn't have a variable assigned to it.

JessicaFrank commented 1 year ago

I've been trying to build a simpler daisy chain example that breaks like the NY one, but this version has multiple jumps and is still working as expected in a2j.org.

Daisy Chain Jumps (1).zip

mikemitchel commented 1 year ago

The race condition cause seems to be confirmed by running Steps-test.js for the viewer. The first 2 tests pass, but the 3rd fails as it has an interview model instead of a list of steps. When you run the 3rd test by itself, it passes. The likely cause is the CanJS bindings/async data synchronization being too slow or suffering from infinite loops. Likely solution is to make sure there is no :bind being used in .stache files, and should only use :to or :from stache bindings to control the flow of data. The sortedAnswers list is also a likely culprit as it is created/sorted each time a page is navigated.

JessicaFrank commented 1 year ago

Note to the reporting author and LHI team:

Here's where I think we are on the Firefox bug reported around the NY interviews. I'm going to focus on just one to simplify our discussions (and I've been testing/able to replicate in that one interview) - the Small Estate Settlement.

The initial report was that the answers were being lost, causing errors in the assembled document that were contingent on true false variables. The underlying answer file had [Unassigned Variable 1] in it in multiple places. Also reports of the interview running slowly in Firefox and having trouble saving answers when testing it on a2jauthor.org in Preview mode.

The good news is that I think we've identified what's happening and we've seen this type of bug before with the IE browser in 2017 and some NY interviews. I think the issue could technically happen in any large interview, but that it might be isolated to a unique way of authoring that is present in the NY interviews.

The bad news is we haven't fixed it yet. We hired our old Bitovi developer Mike to work on it, but he's only on for a set number of hours and debugging the issue has eaten up half of his time. We're trying some fixes today for a couple hours to see if we can knock it out. I'll update you all on the progress of the bug fix.

Long story long, but here's what I think the underlying bug is --- it's a timing issue. The Small Estate Settlement has multiple pages that only contain BEFORE logic. They don't display anything to the end user and push everyone on to another page. I counted 5 in my testing passes where I was able to replicate the [Unassigned Variable 1]. I think these are remnants of authoring in A2J Author 4 which had more limited logic capacity. So essentially the user sees a page, interacts with it, clicks the button to continue, then the viewer fires the goto logic underlying the buttons, sends to a logic only page, and immediately hits logic again to GOTO a different 3rd page. Then the user is shown the 3rd page and interacts with it. This happens multiple times in the interview.

So the A2J Viewer is rapidly jumping the end user to different pages after a single button click. Something in how Firefox is interpreting these jumps is getting the Viewer out of sync. This only presented itself after the Firefox browser update in December 2022 (as far as we know). After these Logic only pages, the viewer will hit a page with variables, display the fields to the user, store the user's answers, but then not realize that there is a variable name assigned to that field. The out of sync Viewer thinks there is a field with no assigned variable. So it defaults to assigning it [Unassigned Variable 1] and storing the user's answer in that variable instead of whatever the actual variable was. We built in this fallback of [Unassigned Variable 1] because authors would forget to put variables in fields and the resulting /undefined variable name would crash the HotDocs assembly. So we're falling back on a failsafe but inappropriately so due to a timing issue. Then when the document is assembled, HotDocs is looking for some other variable and not finding it or any values associated with it because some values have gone into [Unassigned variable 1] instead.

The faster you run through the interview, the worse the out of sync behavior can get. Which is why Sun's testing showed an answer file full of [Unassigned Variable 1]. I was testing and taking notes at the same time, so my answer file only had 1 [Unassigned Variable 1] in it.

While we're working on a fix to the timing issue, there is a work around. Those logic only pages aren't necessary any more in A2J Author. That same logic could be contained in prior pages to remove these "daisy chain" logic only pages. I think that would solve the timing issue.

JessicaFrank commented 1 year ago

Author response:

Hi all!

Oh!!! That makes sense… I still put in those logic only screens because it’s cleaner (in my head) to have everything go to the same place in case I want to change the destination window at some point – so instead of changing it in 5 places, I only have to change it in the forwarding logic window. Or I use them to keep a group of screens together. Also, I like to sort out the variables in a separate window before figuring out where the next goto is – it’s easier to go back to that one window again to fix the logic if I have to.

Some of my interviews are really big with lots of variables to wrangle. I’ll do a review of the interviews that are buggy in Firefox and see if it’s something I can take out without making the whole thing fall apart

tobiasnteireho commented 5 months ago

assuming related to bad firefox version as in #241