From 663c220eaf37a14e599e6468be97fe57df5203f7 Mon Sep 17 00:00:00 2001
From: Phoenix Eliot <phoenixjeliot@gmail.com>
Date: Tue, 23 Aug 2016 16:53:54 -0700
Subject: [PATCH] Show wev-dev iFrame error messages like Aether's This heavily
 refactors SpellView and adds infrastructure for receiving and reporting
 Errors raised by the web-dev iFrame. The web-dev error system, the Aether
 error system, and the Ace html-worker avoid disturbing each others'
 errors/annotations (though currently Aether+web-dev errors won't coexist),
 and they clear/update their own asynchronously.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Show web-dev iFrame errors as Ace annotations

Add functional error banners (with poor messages)

Improve error banners, don't allow duplicate Problems

Refactor setAnnotations override

Convert all constructor calls for Problems

Add comments, clean up

Clean up

Don't clear things unnecessarily

Clean up error message sending from iFrame

Add web-dev:error schema

Clarify error message attributes

Refactor displaying AetherProblems

Refactor displaying user problem banners

Refactor onWebDevError

Set ace styles on updating @problems

Clean up, fix off-by-1 error

Add comment

Show stale web-dev errors differently
Some web-dev errors are generated by "stale" code — code that's still running in the iFrame but doesn't have the player's recent changes.
This shows those errors differently than if they weren't "stale", and suggests they re-run their code.

Hook up web-dev event schema

Destroy ignored duplicate problems

Functionalize a bit of stuff

Fix ProblemAlertView never loading
---
 app/assets/javascripts/web-dev-listener.js    |  17 ++-
 app/core/initialize.coffee                    |   1 +
 app/schemas/subscriptions/tome.coffee         |   2 +-
 app/schemas/subscriptions/web-dev.coffee      |   9 ++
 app/styles/play/level/tome/spell.sass         |   4 -
 app/views/play/level/WebSurfaceView.coffee    |   5 +-
 app/views/play/level/tome/Problem.coffee      |  93 ++++++++++---
 .../play/level/tome/ProblemAlertView.coffee   |  18 +--
 app/views/play/level/tome/Spell.coffee        |   1 +
 app/views/play/level/tome/SpellView.coffee    | 130 +++++++++++++-----
 server/commons/database.coffee                |   1 -
 .../views/play/level/tome/Problem.spec.coffee |   6 +-
 12 files changed, 203 insertions(+), 84 deletions(-)
 create mode 100644 app/schemas/subscriptions/web-dev.coffee

diff --git a/app/assets/javascripts/web-dev-listener.js b/app/assets/javascripts/web-dev-listener.js
index a1802604b..dc0e5f67d 100644
--- a/app/assets/javascripts/web-dev-listener.js
+++ b/app/assets/javascripts/web-dev-listener.js
@@ -1,5 +1,11 @@
 // TODO: don't serve this script from codecombat.com; serve it from a harmless extra domain we don't have yet.
 
+var lastSource = null;
+var lastOrigin = null;
+window.onerror = function(message, url, line, column, error){
+  console.log("User script error on line " + line + ", column " + column + ": ", error);
+  lastSource.postMessage({ type: 'error', message: message, url: url, line: line, column: column }, lastOrigin);
+}
 window.addEventListener('message', receiveMessage, false);
 
 var concreteDom;
@@ -30,8 +36,9 @@ function receiveMessage(event) {
         console.log('Ignoring message from bad origin:', origin);
         return;
     }
+    lastOrigin = origin;
     var data = event.data;
-    var source = event.source;
+    var source = lastSource = event.source;
     switch (data.type) {
     case 'create':
         create(_.pick(data, 'dom', 'styles', 'scripts'));
@@ -79,12 +86,8 @@ function replaceNodes(selector, newNodes){
     $newNodes.attr('for', firstNode.attr('for'));
     
     newFirstNode = $newNodes[0];
-    try {
-      firstNode.replaceWith(newFirstNode); // Removes newFirstNode from its array (!!)
-    } catch (e) {
-      console.log('Failed to update some nodes:', e);
-    }
-    
+    firstNode.replaceWith(newFirstNode); // Removes newFirstNode from its array (!!)
+
     $(newFirstNode).after($newNodes);
 }
 
diff --git a/app/core/initialize.coffee b/app/core/initialize.coffee
index b81841a6b..cab60e5c2 100644
--- a/app/core/initialize.coffee
+++ b/app/core/initialize.coffee
@@ -13,6 +13,7 @@ channelSchemas =
   'tome': require 'schemas/subscriptions/tome'
   'god': require 'schemas/subscriptions/god'
   'scripts': require 'schemas/subscriptions/scripts'
+  'web-dev': require 'schemas/subscriptions/web-dev'
   'world': require 'schemas/subscriptions/world'
 
 definitionSchemas =
diff --git a/app/schemas/subscriptions/tome.coffee b/app/schemas/subscriptions/tome.coffee
index cff846e0c..d9d5f10d3 100644
--- a/app/schemas/subscriptions/tome.coffee
+++ b/app/schemas/subscriptions/tome.coffee
@@ -89,7 +89,7 @@ module.exports =
   'tome:problems-updated': c.object {title: 'Problems Updated', description: 'Published when problems have been updated', required: ['spell', 'problems', 'isCast']},
     spell: {type: 'object'}
     problems: {type: 'array'}
-    isCast: {type: 'boolean'}
+    isCast: {type: 'boolean', description: 'Whether the code has been Run yet. Sometimes determines if error displays as just annotation or as full banner.'}
 
   'tome:change-language': c.object {title: 'Tome Change Language', description: 'Published when the Tome should update its programming language', required: ['language']},
     language: {type: 'string'}
diff --git a/app/schemas/subscriptions/web-dev.coffee b/app/schemas/subscriptions/web-dev.coffee
new file mode 100644
index 000000000..4bd8c07b7
--- /dev/null
+++ b/app/schemas/subscriptions/web-dev.coffee
@@ -0,0 +1,9 @@
+c = require 'schemas/schemas'
+
+module.exports =
+  'web-dev:error': c.object {title: 'Web Dev Error', description: 'Published when an uncaught error occurs in the web-dev iFrame', required: []},
+    message: { type: 'string' }
+    url: { type: 'string', description: 'URL of the host iFrame' }
+    line: { type: 'integer', description: 'Line number of the start of the code that threw the exception (relative to its <script> tag!)' }
+    column: { type: 'integer', description: 'Column number of the start of the code that threw the exception' }
+    error: { type: 'string', description: 'The .toString of the originally thrown exception' }
diff --git a/app/styles/play/level/tome/spell.sass b/app/styles/play/level/tome/spell.sass
index b3928a4b3..ba6611951 100644
--- a/app/styles/play/level/tome/spell.sass
+++ b/app/styles/play/level/tome/spell.sass
@@ -188,10 +188,6 @@
     .ace_gutter-cell.ace_error
       background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAABHNCSVQICAgIfAhkiAAAAAlwSFlzAAALEgAACxIB0t1+/AAAABx0RVh0U29mdHdhcmUAQWRvYmUgRmlyZXdvcmtzIENTM5jWRgMAAAAVdEVYdENyZWF0aW9uIFRpbWUAMi8xNy8wOCCcqlgAAAQRdEVYdFhNTDpjb20uYWRvYmUueG1wADw/eHBhY2tldCBiZWdpbj0iICAgIiBpZD0iVzVNME1wQ2VoaUh6cmVTek5UY3prYzlkIj8+Cjx4OnhtcG1ldGEgeG1sbnM6eD0iYWRvYmU6bnM6bWV0YS8iIHg6eG1wdGs9IkFkb2JlIFhNUCBDb3JlIDQuMS1jMDM0IDQ2LjI3Mjk3NiwgU2F0IEphbiAyNyAyMDA3IDIyOjExOjQxICAgICAgICAiPgogICA8cmRmOlJERiB4bWxuczpyZGY9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkvMDIvMjItcmRmLXN5bnRheC1ucyMiPgogICAgICA8cmRmOkRlc2NyaXB0aW9uIHJkZjphYm91dD0iIgogICAgICAgICAgICB4bWxuczp4YXA9Imh0dHA6Ly9ucy5hZG9iZS5jb20veGFwLzEuMC8iPgogICAgICAgICA8eGFwOkNyZWF0b3JUb29sPkFkb2JlIEZpcmV3b3JrcyBDUzM8L3hhcDpDcmVhdG9yVG9vbD4KICAgICAgICAgPHhhcDpDcmVhdGVEYXRlPjIwMDgtMDItMTdUMDI6MzY6NDVaPC94YXA6Q3JlYXRlRGF0ZT4KICAgICAgICAgPHhhcDpNb2RpZnlEYXRlPjIwMDgtMDMtMjRUMTk6MDA6NDJaPC94YXA6TW9kaWZ5RGF0ZT4KICAgICAgPC9yZGY6RGVzY3JpcHRpb24+CiAgICAgIDxyZGY6RGVzY3JpcHRpb24gcmRmOmFib3V0PSIiCiAgICAgICAgICAgIHhtbG5zOmRjPSJodHRwOi8vcHVybC5vcmcvZGMvZWxlbWVudHMvMS4xLyI+CiAgICAgICAgIDxkYzpmb3JtYXQ+aW1hZ2UvcG5nPC9kYzpmb3JtYXQ+CiAgICAgIDwvcmRmOkRlc2NyaXB0aW9uPgogICA8L3JkZjpSREY+CjwveDp4bXBtZXRhPgogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAKICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIDUdUmQAAAD5SURBVDiNpZMxagMxEEWfgiCXcB3IbXwD7zbaM0nNyjdIl1O4Dk7pbsslEFbEZFKsJsiJrGDy4YM0M//zRyAoINAJyB8cS43RwwIdMFrvaeE8DADxXqQ3Jstn6GaQ5L3M0GQxsyaZoJtA3r2XCS6o+FkvZkdOIG/eywl+UVHrqcYm4BNIjb1rPdXYBTivj3gVtZ5q/p8gAfPhcLOBamzKcW41UI1dgA/qez4bU6muUE0zwVYEgKeKkWruEnTHENg4R8pFZblCyY1zHEMgQTQAe9gB8cE5XkO4GhugmIk76L+z+Wzy6FzT4CWLXf5MF8upSdMB4gC9Xr4AiezTJHGxdq0AAAAASUVORK5CYII=)
 
-    // NOTE! This hides all info annotations because removing specific annotations with listeners means they flicker. This hides that flicker. see: SpellView.onChangeAnnotation
-    .ace_gutter-cell.ace_info
-      background-image: none
-
     .ace_marker-layer
       .ace_bracket
         // Override faint gray
diff --git a/app/views/play/level/WebSurfaceView.coffee b/app/views/play/level/WebSurfaceView.coffee
index 56d2d7559..d737a662d 100644
--- a/app/views/play/level/WebSurfaceView.coffee
+++ b/app/views/play/level/WebSurfaceView.coffee
@@ -88,8 +88,11 @@ module.exports = class WebSurfaceView extends CocoView
     switch event.data.type
       when 'goals-updated'
         Backbone.Mediator.publish 'god:new-html-goal-states', goalStates: event.data.goalStates, overallStatus: event.data.overallStatus
+      when 'error'
+        # NOTE: The line number in this is relative to the script tag, not the user code. The offset is added in SpellView.
+        Backbone.Mediator.publish 'web-dev:error', _.pick(event.data, ['message', 'line', 'column', 'url'])
       else
-        console.warn 'Unknown message type', event.data.type, 'for message', e, 'from origin', origin
+        console.warn 'Unknown message type', event.data.type, 'for message', event, 'from origin', origin
 
   destroy: ->
     window.removeEventListener 'message', @onIframeMessage
diff --git a/app/views/play/level/tome/Problem.coffee b/app/views/play/level/tome/Problem.coffee
index 9b5b22fa8..1ebbfeab2 100644
--- a/app/views/play/level/tome/Problem.coffee
+++ b/app/views/play/level/tome/Problem.coffee
@@ -1,43 +1,96 @@
 Range = ace.require('ace/range').Range
 
+# This class can either wrap an AetherProblem,
+# or act as a general runtime error container for web-dev iFrame errors.
+# TODO: Use subclasses? Might need a factory pattern for that (bleh)
 module.exports = class Problem
   annotation: null
   markerRange: null
-  constructor: (@aether, @aetherProblem, @ace, isCast=false, @levelID) ->
-    @buildAnnotation()
-    @buildMarkerRange() if isCast
+  # Construction with AetherProblem will include all but `error`
+  # Construction with a standard error will have `error`, `isCast`, `levelID`, `ace`
+  constructor: ({ @aether, @aetherProblem, @ace, isCast=false, @levelID, error, userCodeHasChangedSinceLastCast }) ->
+    if @aetherProblem
+      @annotation = @buildAnnotationFromAetherProblem(@aetherProblem)
+      { @lineMarkerRange, @textMarkerRange } = @buildMarkerRangesFromAetherProblem(@aetherProblem) if isCast
+
+      { @level, @range, @message, @hint, @userInfo } = @aetherProblem
+      { @row, @column: col } = @aetherProblem.range?[0]
+      @createdBy = 'aether'
+    else
+      unless userCodeHasChangedSinceLastCast
+        @annotation = @buildAnnotationFromWebDevError(error)
+        { @lineMarkerRange, @textMarkerRange } = @buildMarkerRangesFromWebDevError(error)
+
+      @level = 'error'
+      @row = error.line
+      @column = error.column
+      @message = error.message or 'Unknown Error'
+      if error.line and not userCodeHasChangedSinceLastCast
+        @message = "Line #{error.line + 1}: " + @message # Ace's gutter numbers are 1-indexed but annotation.rows are 0-indexed
+      if userCodeHasChangedSinceLastCast
+        @hint = "This error was generated by old code — Try running your new code first."
+      else
+        @hint = undefined
+      @userInfo = undefined
+      @createdBy = 'web-dev-iframe'
+      # TODO: Include runtime/transpile error types depending on something?
+
     # TODO: get ACE screen line, too, for positioning, since any multiline "lines" will mess up positioning
     Backbone.Mediator.publish("problem:problem-created", line: @annotation.row, text: @annotation.text) if application.isIPadApp
 
+  isEqual: (problem) ->
+    _.all ['row', 'column', 'level', 'column', 'message', 'hint'], (attr) =>
+      @[attr] is problem[attr]
+
   destroy: ->
     @removeMarkerRanges()
     @userCodeProblem.off() if @userCodeProblem
 
-  buildAnnotation: ->
-    return unless @aetherProblem.range
-    text = @aetherProblem.message.replace /^Line \d+: /, ''
-    start = @aetherProblem.range[0]
-    @annotation =
+  buildAnnotationFromWebDevError: (error) ->
+    {
+      row: error.line
+      column: error.column
+      raw: error.message
+      text: error.message
+      type: 'error'
+      createdBy: 'web-dev-iframe'
+    }
+
+  buildAnnotationFromAetherProblem: (aetherProblem) ->
+    return unless aetherProblem.range
+    text = aetherProblem.message.replace /^Line \d+: /, ''
+    start = aetherProblem.range[0]
+    {
       row: start.row,
       column: start.col,
       raw: text,
       text: text,
       type: @aetherProblem.level ? 'error'
       createdBy: 'aether'
+    }
 
-  buildMarkerRange: ->
-    return unless @aetherProblem.range
-    [start, end] = @aetherProblem.range
-    textClazz = "problem-marker-#{@aetherProblem.level}"
-    @textMarkerRange = new Range start.row, start.col, end.row, end.col
-    @textMarkerRange.start = @ace.getSession().getDocument().createAnchor @textMarkerRange.start
-    @textMarkerRange.end = @ace.getSession().getDocument().createAnchor @textMarkerRange.end
-    @textMarkerRange.id = @ace.getSession().addMarker @textMarkerRange, textClazz, 'text'
+  buildMarkerRangesFromWebDevError: (error) ->
+    lineMarkerRange = new Range error.line, 0, error.line, 1
+    lineMarkerRange.start = @ace.getSession().getDocument().createAnchor lineMarkerRange.start
+    lineMarkerRange.end = @ace.getSession().getDocument().createAnchor lineMarkerRange.end
+    lineMarkerRange.id = @ace.getSession().addMarker lineMarkerRange, 'problem-line', 'fullLine'
+    textMarkerRange = undefined # We don't get any per-character info from standard errors
+    { lineMarkerRange, textMarkerRange }
+
+  buildMarkerRangesFromAetherProblem: (aetherProblem) ->
+    return {} unless aetherProblem.range
+    [start, end] = aetherProblem.range
+    textClazz = "problem-marker-#{aetherProblem.level}"
+    textMarkerRange = new Range start.row, start.col, end.row, end.col
+    textMarkerRange.start = @ace.getSession().getDocument().createAnchor textMarkerRange.start
+    textMarkerRange.end = @ace.getSession().getDocument().createAnchor textMarkerRange.end
+    textMarkerRange.id = @ace.getSession().addMarker textMarkerRange, textClazz, 'text'
     lineClazz = "problem-line"
-    @lineMarkerRange = new Range start.row, start.col, end.row, end.col
-    @lineMarkerRange.start = @ace.getSession().getDocument().createAnchor @lineMarkerRange.start
-    @lineMarkerRange.end = @ace.getSession().getDocument().createAnchor @lineMarkerRange.end
-    @lineMarkerRange.id = @ace.getSession().addMarker @lineMarkerRange, lineClazz, 'fullLine'
+    lineMarkerRange = new Range start.row, start.col, end.row, end.col
+    lineMarkerRange.start = @ace.getSession().getDocument().createAnchor lineMarkerRange.start
+    lineMarkerRange.end = @ace.getSession().getDocument().createAnchor lineMarkerRange.end
+    lineMarkerRange.id = @ace.getSession().addMarker lineMarkerRange, lineClazz, 'fullLine'
+    { lineMarkerRange, textMarkerRange }
 
   removeMarkerRanges: ->
     if @textMarkerRange
diff --git a/app/views/play/level/tome/ProblemAlertView.coffee b/app/views/play/level/tome/ProblemAlertView.coffee
index c9ffc1363..f63e1e13c 100644
--- a/app/views/play/level/tome/ProblemAlertView.coffee
+++ b/app/views/play/level/tome/ProblemAlertView.coffee
@@ -20,10 +20,10 @@ module.exports = class ProblemAlertView extends CocoView
     'click': -> Backbone.Mediator.publish 'tome:focus-editor', {}
 
   constructor: (options) ->
+    @supermodel = options.supermodel # Has to go before super so events are hooked up
     super options
     @level = options.level
     @session = options.session
-    @supermodel = options.supermodel
     if options.problem?
       @problem = options.problem
       @onWindowResize()
@@ -38,31 +38,31 @@ module.exports = class ProblemAlertView extends CocoView
   afterRender: ->
     super()
     if @problem?
-      @$el.addClass('alert').addClass("alert-#{@problem.aetherProblem.level}").hide().fadeIn('slow')
-      @$el.addClass('no-hint') unless @problem.aetherProblem.hint
+      @$el.addClass('alert').addClass("alert-#{@problem.level}").hide().fadeIn('slow')
+      @$el.addClass('no-hint') unless @problem.hint
       @playSound 'error_appear'
 
   setProblemMessage: ->
     if @problem?
       format = (s) -> marked(s.replace(/</g, '&lt;').replace(/>/g, '&gt;')) if s?
-      message = @problem.aetherProblem.message
+      message = @problem.message
       # Add time to problem message if hint is for a missing null check
       # NOTE: This may need to be updated with Aether error hint changes
-      if @problem.aetherProblem.hint? and /(?:null|undefined)/.test @problem.aetherProblem.hint
-        age = @problem.aetherProblem.userInfo?.age
+      if @problem.hint? and /(?:null|undefined)/.test @problem.hint
+        age = @problem.userInfo?.age
         if age?
           if /^Line \d+:/.test message
             message = message.replace /^(Line \d+)/, "$1, time #{age.toFixed(1)}"
           else
             message = "Time #{age.toFixed(1)}: #{message}"
       @message = format message
-      @hint = format @problem.aetherProblem.hint
+      @hint = format @problem.hint
 
   onShowProblemAlert: (data) ->
     return unless $('#code-area').is(":visible")
     if @problem?
-      if @$el.hasClass "alert-#{@problem.aetherProblem.level}"
-        @$el.removeClass "alert-#{@problem.aetherProblem.level}"
+      if @$el.hasClass "alert-#{@problem.level}"
+        @$el.removeClass "alert-#{@problem.level}"
       if @$el.hasClass "no-hint"
         @$el.removeClass "no-hint"
     @problem = data.problem
diff --git a/app/views/play/level/tome/Spell.coffee b/app/views/play/level/tome/Spell.coffee
index 5b8f14532..1db486b82 100644
--- a/app/views/play/level/tome/Spell.coffee
+++ b/app/views/play/level/tome/Spell.coffee
@@ -145,6 +145,7 @@ module.exports = class Spell
       @thang?.aether.transpile source
     null
 
+  # NOTE: By default, I think this compares the current source code with the source *last saved to the server* (not the last time it was run)
   hasChanged: (newSource=null, currentSource=null) ->
     (newSource ? @originalSource) isnt (currentSource ? @source)
 
diff --git a/app/views/play/level/tome/SpellView.coffee b/app/views/play/level/tome/SpellView.coffee
index fe8d07c0a..2dc095b82 100644
--- a/app/views/play/level/tome/SpellView.coffee
+++ b/app/views/play/level/tome/SpellView.coffee
@@ -49,10 +49,12 @@ module.exports = class SpellView extends CocoView
     'tome:insert-snippet': 'onInsertSnippet'
     'tome:spell-beautify': 'onSpellBeautify'
     'tome:maximize-toggled': 'onMaximizeToggled'
+    'tome:problems-updated': 'onProblemsUpdated'
     'script:state-changed': 'onScriptStateChange'
     'playback:ended-changed': 'onPlaybackEndedChanged'
     'level:contact-button-pressed': 'onContactButtonPressed'
     'level:show-victory': 'onShowVictory'
+    'web-dev:error': 'onWebDevError'
 
   events:
     'mouseout': 'onMouseOut'
@@ -87,6 +89,14 @@ module.exports = class SpellView extends CocoView
     @destroyAceEditor(@ace)
     @ace = ace.edit @$el.find('.ace')[0]
     @aceSession = @ace.getSession()
+    # Override setAnnotations so the Ace html worker doesn't clobber our annotations
+    @reallySetAnnotations = @aceSession.setAnnotations.bind(@aceSession)
+    @aceSession.setAnnotations = (annotations) =>
+      previousAnnotations = @aceSession.getAnnotations()
+      newAnnotations = _.filter previousAnnotations, (annotation) -> annotation.createdBy? # Keep the ones we generated
+        .concat _.reject annotations, (annotation) -> # Ignore this particular info-annotation the html worker generates
+          annotation.text is 'Start tag seen without seeing a doctype first. Expected e.g. <!DOCTYPE html>.'
+      @reallySetAnnotations newAnnotations
     @aceDoc = @aceSession.getDocument()
     @aceSession.setUseWorker @spell.language in @languagesThatUseWorkers
     @aceSession.setMode utils.aceEditModes[@spell.language]
@@ -94,7 +104,6 @@ module.exports = class SpellView extends CocoView
     @aceSession.setUseWrapMode true
     @aceSession.setNewLineMode 'unix'
     @aceSession.setUseSoftTabs true
-    @aceSession.on 'changeAnnotation', @onChangeAnnotation
     @ace.setTheme 'ace/theme/textmate'
     @ace.setDisplayIndentGuides false
     @ace.setShowPrintMargin false
@@ -670,7 +679,10 @@ module.exports = class SpellView extends CocoView
     cast = @$el.parent().length
     @recompile cast, e.realTime
     @focus() if cast
-    @updateHTML create: true if @options.level.isType('web-dev')
+    if @options.level.isType('web-dev')
+      @sourceAtLastCast = @getSource()
+      @ace.setStyle 'spell-cast'
+      @updateHTML create: true
 
   onCodeReload: (e) ->
     return unless e.spell is @spell or not e.spell
@@ -736,6 +748,11 @@ module.exports = class SpellView extends CocoView
   onCursorActivity: =>  # Used to refresh autocast delay; doesn't do anything at the moment.
 
   updateHTML: (options={}) =>
+    # TODO: Merge with onSpellChanged
+    # NOTE: Consider what goes in onManualCast only
+    if @spell.hasChanged(@spell.getSource(), @sourceAtLastCast)
+      @ace.unsetStyle 'spell-cast' # NOTE: Doesn't do anything for web-dev as of this writing, including for consistency
+    @clearWebDevErrors()
     Backbone.Mediator.publish 'tome:html-updated', html: @spell.constructHTML(@getSource()), create: Boolean(options.create)
 
   # Design for a simpler system?
@@ -770,6 +787,7 @@ module.exports = class SpellView extends CocoView
       # Now that that's figured out, perform the update.
       # The web worker Aether won't track state, so don't have to worry about updating it
       finishUpdatingAether = (aether) =>
+        @clearAetherDisplay() # In case problems were added since last clearing
         @displayAether aether, codeIsAsCast
         @lastUpdatedAetherSpellThang = @spellThang
         @guessWhetherFinished aether if fromCodeChange
@@ -796,57 +814,92 @@ module.exports = class SpellView extends CocoView
       else
         finishUpdatingAether(aether)
 
-  # NOTE! Because this alone causes the doctype annotation to flicker,
-  # all info annotations have been hidden with CSS in spell.sass
-  # If we ever want info annotations back, we need to remove that.
-  #
-  # This function itself removes the unwanted annotations on a later tick.
-  onChangeAnnotation: (event, session) ->
-    unfilteredAnnotations = session.getAnnotations()
-    filteredAnnotations = _.reject unfilteredAnnotations, (annotation) ->
-      annotation.text is 'Start tag seen without seeing a doctype first. Expected e.g. <!DOCTYPE html>.'
-    if filteredAnnotations.length < unfilteredAnnotations.length
-      session.setAnnotations(filteredAnnotations)
-
-  # Clear annotations and highlights generated by Aether, but not by the ACE worker
+  # Each problem-generating piece (aether, web-dev, ace html worker) clears its own problems/annotations
   clearAetherDisplay: ->
-    problem.destroy() for problem in @problems
-    @problems = []
-    nonAetherAnnotations = _.reject @aceSession.getAnnotations(), (annotation) -> annotation.createdBy is 'aether'
-    @aceSession.setAnnotations nonAetherAnnotations
+    @clearProblemsCreatedBy 'aether'
     @highlightCurrentLine {}  # This'll remove all highlights
 
+  clearWebDevErrors: ->
+    @clearProblemsCreatedBy 'web-dev-iframe'
+
+  clearProblemsCreatedBy: (createdBy) ->
+    nonAetherAnnotations = _.reject @aceSession.getAnnotations(), (annotation) -> annotation.createdBy is createdBy
+    @reallySetAnnotations nonAetherAnnotations
+
+    problemsToClear = _.filter @problems, (p) -> p.createdBy is createdBy
+    problemsToClear.forEach (problem) -> problem.destroy()
+    @problems = _.difference @problems, problemsToClear
+    Backbone.Mediator.publish 'tome:problems-updated', spell: @spell, problems: @problems, isCast: false
+
+  convertAetherProblems: (aether, aetherProblems, isCast) ->
+    # TODO: Functional-ify
+    _.unique(aetherProblems, (p) -> p.userInfo?.key).map (aetherProblem) =>
+      new Problem { aether, aetherProblem, @ace, isCast, levelID: @options.levelID }
+
   displayAether: (aether, isCast=false) ->
     @displayedAether = aether
     isCast = isCast or not _.isEmpty(aether.metrics) or _.some aether.getAllProblems(), {type: 'runtime'}
-    problem.destroy() for problem in @problems  # Just in case another problem was added since clearAetherDisplay() ran.
-    @problems = []
     annotations = @aceSession.getAnnotations()
-    seenProblemKeys = {}
-    for aetherProblem, problemIndex in aether.getAllProblems()
-      continue if key = aetherProblem.userInfo?.key and key of seenProblemKeys
-      seenProblemKeys[key] = true if key
-      @problems.push problem = new Problem aether, aetherProblem, @ace, isCast, @options.levelID
-      if isCast and problemIndex is 0
-        if problem.aetherProblem.range?
-          lineOffsetPx = 0
-          for i in [0...problem.aetherProblem.range[0].row]
-            lineOffsetPx += @aceSession.getRowLength(i) * @ace.renderer.lineHeight
-          lineOffsetPx -= @ace.session.getScrollTop()
-        Backbone.Mediator.publish 'tome:show-problem-alert', problem: problem, lineOffsetPx: Math.max lineOffsetPx, 0
-      @saveUserCodeProblem(aether, aetherProblem) if isCast
-      annotations.push problem.annotation if problem.annotation
+
+    newProblems = @convertAetherProblems(aether, aether.getAllProblems(), isCast)
+    annotations.push problem.annotation for problem in newProblems when problem.annotation
+    if isCast
+      @displayProblemBanner(newProblems[0]) if newProblems[0]
+      @saveUserCodeProblem(aether, problem.aetherProblem) for problem in newProblems
+    @problems = @problems.concat(newProblems)
+
     @aceSession.setAnnotations annotations
     @highlightCurrentLine aether.flow unless _.isEmpty aether.flow
     #console.log '  and we could do the metrics', aether.metrics unless _.isEmpty aether.metrics
     #console.log '  and we could do the style', aether.style unless _.isEmpty aether.style
     #console.log '  and we could do the visualization', aether.visualization unless _.isEmpty aether.visualization
-    # Could use the user-code-problem style... or we could leave that to other places.
-    @ace[if @problems.length then 'setStyle' else 'unsetStyle'] 'user-code-problem'
-    @ace[if isCast then 'setStyle' else 'unsetStyle'] 'spell-cast'
     Backbone.Mediator.publish 'tome:problems-updated', spell: @spell, problems: @problems, isCast: isCast
     @ace.resize()
 
+  # Tell ProblemAlertView to display this problem (only)
+  displayProblemBanner: (problem) ->
+    lineOffsetPx = 0
+    if problem.row?
+      for i in [0...problem.row]
+        lineOffsetPx += @aceSession.getRowLength(i) * @ace.renderer.lineHeight
+      lineOffsetPx -= @ace.session.getScrollTop()
+    Backbone.Mediator.publish 'tome:show-problem-alert', problem: problem, lineOffsetPx: Math.max lineOffsetPx, 0
+
+  # Gets the number of lines before the start of <script> content in the usercode
+  # Because Errors report their line number relative to the <script> tag
+  linesBeforeScript: (html) ->
+    # TODO: refactor, make it work with multiple scripts. What to do when error is in level-creator's code?
+    _.size(html.split('<script>')[0].match(/\n/g))
+
+  addAnnotation: (annotation) ->
+    annotations = @aceSession.getAnnotations()
+    annotations.push annotation
+    @reallySetAnnotations annotations
+
+  # Handle errors from the web-dev iframe asynchronously
+  onWebDevError: (error) ->
+    # TODO: Refactor this and the Aether problem flow to share as much as possible.
+    # TODO: Handle when the error is in our code, not theirs
+    # Compensate for line number being relative to <script> tag
+    offsetError = _.merge {}, error, { line: error.line + @linesBeforeScript(@getSource()) }
+    userCodeHasChangedSinceLastCast = @spell.hasChanged(@spell.getSource(), @sourceAtLastCast)
+    problem = new Problem({ error: offsetError, @ace, levelID: @options.levelID, userCodeHasChangedSinceLastCast })
+    # Ignore the Problem if we already know about it
+    if _.any(@problems, (preexistingProblem) -> problem.isEqual(preexistingProblem))
+      problem.destroy()
+    else # Ok, the problem is worth keeping
+      @problems.push problem
+      @displayProblemBanner(problem)
+
+      # @saveUserCodeProblem(aether, aetherProblem) # TODO: Enable saving of web-dev user code problems
+      @addAnnotation(problem.annotation) if problem.annotation
+      Backbone.Mediator.publish 'tome:problems-updated', spell: @spell, problems: @problems, isCast: false
+
+  onProblemsUpdated: ({ spell, problems, isCast }) ->
+    # This just handles some ace styles for now; other things handle @problems changes elsewhere
+    @ace[if problems.length then 'setStyle' else 'unsetStyle'] 'user-code-problem'
+    @ace[if isCast then 'setStyle' else 'unsetStyle'] 'spell-cast' # Does this still do anything?
+
   saveUserCodeProblem: (aether, aetherProblem) ->
     # Skip duplicate problems
     hashValue = aether.raw + aetherProblem.message
@@ -939,6 +992,7 @@ module.exports = class SpellView extends CocoView
       @spell.thang.aether[key] = value
 
   onSpellChanged: (e) ->
+    # TODO: Merge with updateHTML
     @spellHasChanged = true
 
   onSessionWillSave: (e) ->
diff --git a/server/commons/database.coffee b/server/commons/database.coffee
index 218ec51f9..ceea28b6a 100644
--- a/server/commons/database.coffee
+++ b/server/commons/database.coffee
@@ -206,4 +206,3 @@ module.exports =
       return false if index is -1
       return false if delta.deltaPath[index+1] in ['en', 'en-US', 'en-GB']  # English speakers are most likely just spamming, so always treat those as patches, not saves.
       return true
-
diff --git a/test/app/views/play/level/tome/Problem.spec.coffee b/test/app/views/play/level/tome/Problem.spec.coffee
index 09b96bdbe..84affeb9a 100644
--- a/test/app/views/play/level/tome/Problem.spec.coffee
+++ b/test/app/views/play/level/tome/Problem.spec.coffee
@@ -29,7 +29,7 @@ describe 'Problem', ->
 
   # TODO: Problems are no longer saved when creating Problems; instead it's in SpellView. Update tests?
   xit 'save user code problem', ->
-    new Problem aether, aetherProblem, ace, false, true, levelID
+    new Problem {aether, aetherProblem, ace, isCast: false, levelID}
     expect(jasmine.Ajax.requests.count()).toBe(1)
 
     request = jasmine.Ajax.requests.mostRecent()
@@ -49,7 +49,7 @@ describe 'Problem', ->
 
   xit 'save user code problem no range', ->
     aetherProblem.range = null
-    new Problem aether, aetherProblem, ace, false, true, levelID
+    new Problem {aether, aetherProblem, ace, isCast: false, levelID}
     expect(jasmine.Ajax.requests.count()).toBe(1)
 
     request = jasmine.Ajax.requests.mostRecent()
@@ -73,7 +73,7 @@ describe 'Problem', ->
     aether.raw = "this.say('hi');\nthis.sad\n('bye');"
     aetherProblem.range = [ { row: 1 }, { row: 2 } ]
 
-    new Problem aether, aetherProblem, ace, false, true, levelID
+    new Problem {aether, aetherProblem, ace, isCast: false, levelID}
     expect(jasmine.Ajax.requests.count()).toBe(1)
 
     request = jasmine.Ajax.requests.mostRecent()