JS errors from malformed project html no longer disable the Start Over button. #29395
Conversation
| @@ -160,6 +160,12 @@ export default { | |||
| } | |||
| // Unknown elements are expected. Return null because we don't know type. | |||
| if (allowUnknown) { | |||
| console.warn( | |||
There was a problem hiding this comment.
Should we log this to firebase?
There was a problem hiding this comment.
Synced offline: Don't need to log to firehose. The intent of having this warning is to help with diagnosing issues that come in via Zendesk. Issues like this will persist, so we won't need to track a single event in time. Additionally, it's not particularly valuable to track how many times this happens.
| `\nType: ${element.tagName}` + | ||
| `\nId: ${element.id}` + | ||
| `\nClass: ${element.className}` | ||
| ); | ||
| return null; | ||
| } | ||
| throw new Error('unknown element type'); |
There was a problem hiding this comment.
Do we ever want to throw on unknown elements? If not, maybe we don't need the allowUnknown parameter and we can always just console.warn and return null?
There was a problem hiding this comment.
I'll look a bit deeper. It would be nice to remove this throw.
There was a problem hiding this comment.
Took a deeper look - there are a lot of areas that rely on this returning something. Additionally, there are a lot of areas that throw when there is a bad design element. I've created a task to look into a better way to approach our strategy on handling malformed design elements.
https://codedotorg.atlassian.net/browse/STAR-556
apps/src/applab/designMode.js
Outdated
| @@ -1164,7 +1166,7 @@ designMode.parseFromLevelHtml = function(rootEl, allowDragging, prefix) { | |||
| ); | |||
| var children = $(levelDom).children(); | |||
| children.each(function() { | |||
| designMode.parseScreenFromLevelHtml(this, allowDragging, prefix); | |||
| designMode.parseScreenFromLevelHtml(this, allowDragging, prefix, true); | |||
There was a problem hiding this comment.
NIT: add a comment for the boolean parameter:
designMode.parseScreenFromLevelHtml(this, allowDragging, prefix, true /* skipUnknownElements */)
…entType. Added a comment to boolean in designMode.parseFromLevelHtml
https://codedotorg.atlassian.net/browse/STAR-385
In the past, if a student had an error in their project html, this would throw an error when the project was loaded. The click handler for the 'start over'/'version history' gets setup after this error is thrown. Because of this, 'start over'/'version history' were broken. A student could never get out of this broken state. They couldn't manually fix their html nor could they restart the level. This fixes that issue by logging rather than throwing if a student's project is in a bad state.
In this case, the image on the screen is wrapped in a paragraph tag (which is invalid in AppLab).
