ERG widget makes Chrome tab unresponsive for some material types

BobBooth commented 6 years ago

Bug: I used the ERG widget to make a series of outputs, testing various of the material types.

Most types worked well, but some froze the app, made the tab go unresponsive.

Repro steps: Add widget to web app, save, launch. Open widget, zoom to a test area, Add a point by clicking on the map. Select a material type. Specify large or small, time of day, wind direction, fire isolation on. Click Create Zones.

Of 21 materials tested, 3 caused app to lock up. Fails: Organic Peroxide (Heat, contamination, and Friction) Adsorbed Gasses Toxic Adsorbed Gasses Toxic (may also be flammable)

Works: Heavy Crude LPG AC (Weaponized Hydrogen Cyanide) Acetyl bromide Aluminum Bromide Boron Tribromide Boron Tribromide- land Bromine Bromine Chloride Acetyl iodide (when spilled in water) Acetyl Chloride admasite Arsenic CL Arsenic 3CL Arsine DA (weaponized) 2-Chloroethanol 3,5-Dichloro-2,4,6-trifluoropyridine

It would be interesting to compare the items that fail to the ones that work - is there some difference in the table they reference?

Repo Steps or Enhancement details

Testing on arcgis-web-appbuilder-2.8-snapshot from 3/5/18 Chrome

Screenshots

BobBooth commented 6 years ago

@kgonzago - ERG issue found testing for solutions-erg-widget/issues/87

BobBooth commented 6 years ago

Perhaps it has something to do with having "IDNum": 0, That seems to be something the ones that failed share. https://github.com/Esri/solutions-erg-widget/blob/dev/EmergencyResponseGuide/guide/materials.json

BobBooth commented 6 years ago

@kgonzago - I had one tab lock up to the point that I had to kill it from Task Manager. If a chrome.DMP file would be useful to you I can share.

BobBooth commented 6 years ago

@adgiles - FYI

BobBooth commented 6 years ago

I suspect that it is the "IDNum":0 issue. There are 63 instances in the https://github.com/Esri/solutions-erg-widget/blob/dev/EmergencyResponseGuide/guide/materials.json file that match this pattern. I went to the next one on the list (line 947) and tried Molten Aluminum... it locked up the app too.

ACueva commented 6 years ago

Adding showstopper since application becomes unresponsive.

BobBooth commented 6 years ago

This is the Firefox Console output starting at the CreateERGButtonClicked event to where I terminated it after it hung.

adgiles commented 6 years ago

I thought it was only materials that contained the word substances that had a guide number of 0, but it looks like this is incorrect. This is a fairly quick fix. Line 1005 just needs changing from:

if(this._selectedMaterial.Material.includes('Substances') || this._selectedMaterial.BLEVE) {

to:

if(this._selectedMaterial.IDNum === 0 || this._selectedMaterial.BLEVE) {

BobBooth commented 6 years ago

@kgonzago - would you give this a shot? Thanks, Bob

kgonzago commented 6 years ago

@BobBooth Working on an existing issue with D&D and will pivot to this issue when done.

kgonzago commented 6 years ago

@BobBooth Is this issue in Devtopia as well? If not, would you mind creating one? Thank you.

kgonzago commented 6 years ago

Issue submitted and fix addressed in this PR. Should be in tomorrow's build for testing.

dfoll commented 6 years ago

removed automated checklist test labels because we are not doing automated testing on the widgets
the fix suggested by @adgiles works, however it did not make it into the daily snapshot for 8Mar2018. I tested with Carbon monoxide (refrigerated liquid), Galium and Mercury, and Lithium Ion Batteries

I checked and the correct code is currently in the develop branch, so I'm leaving this here until someone can test from the daily snapshot

dfoll commented 6 years ago

This is not happening on DevExt (9March2018 1142)

updated checklist test to account for materials that have no ID number
updated checklist test to make sure materials with BLEVE isolation zones are working correctly.
removed G-QA Create checklist
verified the issue does not happen on DevExt DevExt (9March2018 1142) also @topowright observed that it does not on his own account on DevExt as well -removing G-QC Verify checklist

Esri / solutions-erg-widget