Merge pull request #496 from hypothesis/turn-on-multiframe

Removing flags for multiple iframe support

Merge pull request #496 from hypothesis/turn-on-multiframe
Removing flags for multiple iframe support
dc4e7e3b · Robert Knight · GitHub · e29a73f0 · b4f15a6f · dc4e7e3b
Commit dc4e7e3b authored Jul 12, 2017 by Robert Knight Committed by GitHub Jul 12, 2017
6 changed files
--- a/gulpfile.js
+++ b/gulpfile.js
@@ -327,7 +327,6 @@ gulp.task('serve-live-reload', ['serve-package'], function () {
  var LiveReloadServer = require('./scripts/gulp/live-reload-server');
  liveReloadServer = new LiveReloadServer(3000, {
    clientUrl: `http://${packageServerHostname()}:3001/hypothesis`,
-    enableMultiFrameSupport: !!process.env.MULTI_FRAME_SUPPORT,
  });
 });


--- a/scripts/gulp/live-reload-server.js
+++ b/scripts/gulp/live-reload-server.js
@@ -70,16 +70,6 @@ function LiveReloadServer(port, config) {
          </html>
        `;
      } else {
-        var multiFrameContent = config.enableMultiFrameSupport ? `
-          <div style="margin: 10px 0 0 75px;">
-            <button id="add-test" style="padding: 0.6em; font-size: 0.75em">Toggle 2nd Frame</button>
-          </div>
-          <div style="margin: 10px 0 0 75px;">
-            <iframe id="iframe1" src="/document/license" style="width: 50%;height: 300px;"></iframe>
-          </div>
-          <div id="iframe2-container" style="margin: 10px 0 0 75px;">
-          </div>` : '';
-
        content = `
          <html>
          <head>
@@ -91,7 +81,14 @@ function LiveReloadServer(port, config) {
              Number of annotations:
              <span data-hypothesis-annotation-count>...</span>
            </div>
-            ${multiFrameContent}
+            <div style="margin: 10px 0 0 75px;">
+              <button id="add-test" style="padding: 0.6em; font-size: 0.75em">Toggle 2nd Frame</button>
+            </div>
+            <div style="margin: 10px 0 0 75px;">
+              <iframe id="iframe1" src="/document/license" style="width: 50%;height: 300px;"></iframe>
+            </div>
+            <div id="iframe2-container" style="margin: 10px 0 0 75px;">
+            </div>
            <pre style="margin: 20px 75px 75px 75px;">${readmeText()}</pre>
            <script>
            var appHost = document.location.hostname;
@@ -103,8 +100,7 @@ function LiveReloadServer(port, config) {
                // Open the sidebar when the page loads
                openSidebar: true,

-                // Needed for multi frame support
-                enableMultiFrameSupport: ${config.enableMultiFrameSupport},
+                // Subframe client embed code reference
                embedScriptUrl: '${config.clientUrl}'
              };
            };

--- a/src/annotator/config/index.js
+++ b/src/annotator/config/index.js
@@ -18,10 +18,10 @@ function configFrom(window_) {
    openSidebar: settings.hostPageSetting('openSidebar', {allowInBrowserExt: true}),
    branding: settings.hostPageSetting('branding'),
    services: settings.hostPageSetting('services'),
-
-    // Needed by the multi-frame feature for now
-    enableMultiFrameSupport: settings.hostPageSetting('enableMultiFrameSupport'),
    embedScriptUrl: settings.hostPageSetting('embedScriptUrl'),
+
+    // Subframe identifier given when a frame is being embedded into
+    // by a top level client
    subFrameIdentifier: settings.hostPageSetting('subFrameIdentifier'),

    // Temporary feature flag override for 1st-party OAuth

--- a/src/annotator/main.js
+++ b/src/annotator/main.js
@@ -51,7 +51,7 @@ $.noConflict(true)(function() {
    delete config.constructor;
  }

-  if (config.enableMultiFrameSupport && config.subFrameIdentifier) {
+  if (config.subFrameIdentifier) {
    Klass = Guest;

    // Other modules use this to detect if this

--- a/src/annotator/plugin/cross-frame.coffee
+++ b/src/annotator/plugin/cross-frame.coffee
@@ -36,10 +36,10 @@ module.exports = class CrossFrame extends Plugin
    this.pluginInit = ->
      onDiscoveryCallback = (source, origin, token) ->
        bridge.createChannel(source, origin, token)
+
      discovery.startDiscovery(onDiscoveryCallback)

-      if config.enableMultiFrameSupport
-        frameObserver.observe(_injectToFrame, _iframeUnloaded);
+      frameObserver.observe(_injectToFrame, _iframeUnloaded);

    this.destroy = ->
      # super doesnt work here :(

--- a/src/annotator/test/integration/multi-frame-test.js
+++ b/src/annotator/test/integration/multi-frame-test.js
@@ -40,7 +40,6 @@ describe('CrossFrame multi-frame scenario', function () {

    options = {
      config: {
-        enableMultiFrameSupport: true,
        embedScriptUrl: 'data:,', // empty data uri
      },
      on: sandbox.stub(),