Fix frame sizing issue and handle untagged images (#237)

* Fix frame sizing issue and handle untagged images

* Handle case where image is cached and loads immediately

OpenOversight/app/static/js/tag.js

@@ -1,26 +1,48 @@
+function isNumeric(value) {
+    return /^\d+$/.test(value)
+}
+
 $(document).ready(function () {
     const img = $("#face-img")
     const frame = $("#face-tag-frame")
 
-    // To prevent hi-res images from taking over the entire page, the image is being shown
-    // in a responsive element. This means we need to compute the tag frame dimensions using
-    // percentages so it will scale if the page size changes.
-
-    // To do this, we're dividing the tag dimensions by the image's actual width/height and
-    // multiplying by 100.
-    const tagWidth = parseInt(img.data("width")) / img[0].naturalWidth * 100
-    const tagHeight = parseInt(img.data("height")) / img[0].naturalHeight * 100
-    const tagLeft = parseInt(img.data("left")) / img[0].naturalWidth * 100
-    const tagTop = parseInt(img.data("top")) / img[0].naturalHeight * 100
-
-    frame.css({
-        height: tagHeight + "%",
-        width: tagWidth + "%",
-        left: tagLeft + "%",
-        top: tagTop + "%",
-        visibility: "visible",
-    })
-
-    // Make sure wrapper does not expand larger than image
-    $(".face-wrap").css("maxWidth", img[0].naturalWidth + "px")
+    const frameWidth = img.data("width")
+    const frameHeight = img.data("height")
+    const frameLeft = img.data("left")
+    const frameTop = img.data("top")
+
+    if ([frameWidth, frameHeight, frameLeft, frameTop].every(isNumeric)) {
+        img.one("imageLoaded", function () {
+            // To prevent hi-res images from taking over the entire page, the image is being shown
+            // in a responsive element. This means we need to compute the tag frame dimensions
+            // using percentages so it will scale if the page size changes.
+
+            // To do this, we're dividing the tag dimensions by the image's actual width/height and
+            // multiplying by 100.
+            const tagWidth = parseInt(frameWidth) / img[0].naturalWidth * 100
+            const tagHeight = parseInt(frameHeight) / img[0].naturalHeight * 100
+            const tagLeft = parseInt(frameLeft) / img[0].naturalWidth * 100
+            const tagTop = parseInt(frameTop) / img[0].naturalHeight * 100
+
+            frame.css({
+                height: tagHeight + "%",
+                width: tagWidth + "%",
+                left: tagLeft + "%",
+                top: tagTop + "%",
+                visibility: "visible",
+            })
+
+            // Make sure wrapper does not expand larger than image
+            $(".face-wrap").css("maxWidth", img[0].naturalWidth + "px")
+        });
+
+        img.on("load", function () {
+            img.trigger("imageLoaded")
+        })
+
+        // Handle case where image was cached and is loaded before handler is registered
+        if (img[0].complete) {
+            img.trigger("imageLoaded")
+        }
+    }
 });

OpenOversight/app/templates/tag.html

@@ -23,12 +23,19 @@ <h1>Tag {{ face.id }} Detail</h1>
     <div class="col-sm-12 col-md-12">
       <a href="{{ path }}" rel="noopener noreferrer">
         <div class="face-wrap">
+          {% if face.face_position_x is defined
+                and face.face_position_y is defined
+                and face.face_width is defined
+                and face.face_height is defined %}
           <img id="face-img"
             data-left="{{ face.face_position_x }}"
             data-top="{{ face.face_position_y }}"
             data-width="{{ face.face_width }}"
             data-height="{{ face.face_height }}"
             src="{{ path }}" />
+          {% else %}
+          <img id="face-img" src="{{ path }}" />
+          {% endif %}
           <div id="face-tag-frame"></div>
         </div>
       </a>

OpenOversight/tests/conftest.py

@@ -702,7 +702,7 @@ def server(app, request):
     threading.Thread(target=app.run, daemon=True, kwargs={"debug": False}).start()
 
 
-@pytest.fixture
+@pytest.fixture(scope="session")
 def browser(app, request, server):
     # start headless webdriver
     vdisplay = Xvfb()

OpenOversight/tests/test_functional.py

@@ -49,6 +49,16 @@ def wait_for_element(browser, locator, text, timeout=10):
         pytest.fail("Timed out while waiting for element to appear")
 
 
+def wait_for_element_to_be_visible(browser, locator, text, timeout=10):
+    try:
+        element_visible = expected_conditions.visibility_of_element_located(
+            (locator, text)
+        )
+        WebDriverWait(browser, timeout).until(element_visible)
+    except TimeoutException:
+        pytest.fail("Timed out while waiting for element to become visible")
+
+
 @pytest.mark.acceptance
 def test_user_can_load_homepage_and_get_to_form(mockdata, browser):
     browser.get("http://localhost:5000")
@@ -382,6 +392,7 @@ def test_image_classification_and_tagging(mockdata, browser):
     frame = browser.find_element(By.ID, "face-tag-frame")
 
     # 8. Check that the tag frame is fully contained within the image
+    wait_for_element_to_be_visible(browser, By.ID, "face-tag-frame")
     assert image.location["x"] <= frame.location["x"]
     assert image.location["y"] <= frame.location["y"]
     assert (