refactor target related code for pageNavigationWatch (#897)

* refactor target related code for pageNavigationWatch * move watch logic from constructor to the 'start' method * fix https://github.com/DevExpress/testcafe-hammerhead/pull/897/files#r85908853
DevExpress · Nov 1, 2016 · a24a1e4 · a24a1e4
1 parent ca99eb5
commit a24a1e4
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 96 deletions.
diff --git a/src/client/index.js b/src/client/index.js
@@ -196,6 +196,7 @@ class Hammerhead {
         }
 
         this.sandbox.attach(this.win);
+        this.pageNavigationWatch.start();
     }
 }
 

diff --git a/src/client/page-navigation-watch.js b/src/client/page-navigation-watch.js
@@ -2,13 +2,11 @@ import EventEmiter from './utils/event-emitter';
 import { parseProxyUrl } from '../utils/url';
 import { isChangedOnlyHash } from './utils/url';
 import { isShadowUIElement, isAnchorElement, isFormElement, closest } from './utils/dom';
-import ElementSandbox from './sandbox/node/element';
 import * as windowsStorage from './sandbox/windows-storage';
 import domProcessor from './dom-processor/index';
 import nextTick from './utils/next-tick';
 import nativeMethods from './sandbox/native-methods';
 
-
 export default class PageNavigationWatch extends EventEmiter {
     constructor (eventSandbox, codeInstrumentation, elementSandbox) {
         super();
@@ -17,9 +15,9 @@ export default class PageNavigationWatch extends EventEmiter {
 
         this.lastLocationValue = window.location.toString();
 
-        this._locationWatch(codeInstrumentation);
-        this._linkWatch(eventSandbox);
-        this._formWatch(elementSandbox, eventSandbox);
+        this.codeInstrumentation = codeInstrumentation;
+        this.eventSandbox        = eventSandbox;
+        this.elementSandbox      = elementSandbox;
     }
 
     _formWatch (elementSandbox, eventSandbox) {
@@ -59,20 +57,20 @@ export default class PageNavigationWatch extends EventEmiter {
     }
 
     static _getTargetWindow (el) {
-        var targetWindow = window;
-        var target       = nativeMethods.getAttribute.call(el, domProcessor.getStoredAttrName('target')) ||
-                           nativeMethods.getAttribute.call(el, 'target');
-
-        if (target) {
-            if (!ElementSandbox._isKeywordTarget(target))
-                targetWindow = windowsStorage.findByName(target) || window;
-            else if (target === '_top' || target === '_blank')
-                targetWindow = window.top;
-            else if (target === '_parent')
-                targetWindow = window.parent;
+        var target = nativeMethods.getAttribute.call(el, domProcessor.getStoredAttrName('target')) ||
+                     nativeMethods.getAttribute.call(el, 'target') ||
+                     '_self';
+
+        switch (target) {
+            case '_top':
+                return window.top;
+            case '_parent':
+                return window.parent;
+            case '_self':
+                return window;
+            default:
+                return windowsStorage.findByName(target);
         }
-
-        return targetWindow;
     }
 
     _linkWatch (eventSandbox) {
@@ -106,12 +104,9 @@ export default class PageNavigationWatch extends EventEmiter {
 
     _locationWatch (codeInstrumentation) {
         var locationAccessorsInstrumentation = codeInstrumentation.locationAccessorsInstrumentation;
-        var propertyAccessorsInstrumentation = codeInstrumentation.propertyAccessorsInstrumentation;
-
-        var locationChangedHandler = newLocation => this.onNavigationTriggered(newLocation);
+        var locationChangedHandler           = newLocation => this.onNavigationTriggered(newLocation);
 
         locationAccessorsInstrumentation.on(locationAccessorsInstrumentation.LOCATION_CHANGED_EVENT, locationChangedHandler);
-        propertyAccessorsInstrumentation.on(propertyAccessorsInstrumentation.LOCATION_CHANGED_EVENT, locationChangedHandler);
     }
 
     static _onNavigationTriggeredInWindow (win, url) {
@@ -124,7 +119,6 @@ export default class PageNavigationWatch extends EventEmiter {
         /*eslint-enable no-empty */
     }
 
-
     onNavigationTriggered (url) {
         var currentLocation = this.lastLocationValue;
 
@@ -135,4 +129,10 @@ export default class PageNavigationWatch extends EventEmiter {
 
         this.emit(this.PAGE_NAVIGATION_TRIGGERED_EVENT, parseProxyUrl(url).destUrl);
     }
+
+    start () {
+        this._locationWatch(this.codeInstrumentation);
+        this._linkWatch(this.eventSandbox);
+        this._formWatch(this.elementSandbox, this.eventSandbox);
+    }
 }
diff --git a/src/client/sandbox/code-instrumentation/location/wrapper.js b/src/client/sandbox/code-instrumentation/location/wrapper.js
@@ -1,7 +1,13 @@
 import EventEmitter from '../../../utils/event-emitter';
 import createPropertyDesc from '../../../utils/create-property-desc';
 import { get as getDestLocation, getParsed as getParsedDestLocation } from '../../../utils/destination-location';
-import { getProxyUrl, changeDestUrlPart, parseProxyUrl, parseResourceType, isChangedOnlyHash } from '../../../utils/url';
+import {
+    getProxyUrl,
+    changeDestUrlPart,
+    parseProxyUrl,
+    parseResourceType,
+    isChangedOnlyHash
+} from '../../../utils/url';
 import { getDomain, getResourceTypeString } from '../../../../utils/url';
 
 function getLocationUrl (window) {
@@ -17,7 +23,7 @@ export default class LocationWrapper extends EventEmitter {
     constructor (window) {
         super();
 
-        this.CHANGED_EVENT = 'hammerhead|event|location-changed';
+        this.CHANGED_EVENT = 'hammerhead|location-wrapper|changed';
 
         var onChanged            = value => this.emit(this.CHANGED_EVENT, value);
         var locationUrl          = getLocationUrl(window);

diff --git a/src/client/sandbox/code-instrumentation/properties/index.js b/src/client/sandbox/code-instrumentation/properties/index.js
@@ -33,8 +33,6 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
     constructor (nodeMutation, eventSandbox, cookieSandbox, uploadSandbox, shadowUI, storageSandbox) {
         super();
 
-        this.LOCATION_CHANGED_EVENT = 'hammerhead|event|location-changed';
-
         this.nodeMutation          = nodeMutation;
         this.messageSandbox        = eventSandbox.message;
         this.cookieSandbox         = cookieSandbox;

diff --git a/src/client/sandbox/node/element.js b/src/client/sandbox/node/element.js
@@ -546,6 +546,8 @@ export default class ElementSandbox extends SandboxBase {
         window.HTMLTableRowElement.prototype.insertCell    = this.overridedMethods.insertCell;
         window.HTMLFormElement.prototype.submit            = this.overridedMethods.formSubmit;
 
+        this._setValidBrowsingContextOnElementClick(window);
+
         // NOTE: Cookie can be set up for the page by using the request initiated by img.
         // For example: img.src = '<url that responds with the Set-Cookie header>'
         // If img has the 'load' event handler, we redirect the request through proxy.
@@ -574,11 +576,19 @@ export default class ElementSandbox extends SandboxBase {
         el.setAttribute('target', storedAttr || el.target);
     }
 
-    _setValidBrowsingContextOnClick (el) {
-        el.addEventListener('click', () => {
+    _setValidBrowsingContextOnElementClick (window) {
+        this.eventSandbox.listeners.initElementListening(window, ['click']);
+        this.eventSandbox.listeners.addInternalEventListener(window, ['click'], e => {
+            var el = e.target;
+
             if (domUtils.isInputElement(el) && el.form)
                 el = el.form;
 
+            var tagName = domUtils.getTagName(el);
+
+            if (!DomProcessor.isTagWithTargetAttr(tagName))
+                return;
+
             this._ensureTargetContainsExistingBrowsingContext(el);
         });
     }
@@ -618,11 +628,6 @@ export default class ElementSandbox extends SandboxBase {
             case 'base':
                 urlResolver.updateBase(nativeMethods.getAttribute.call(el, domProcessor.getStoredAttrName('href')), this.document);
                 break;
-            case 'a':
-            case 'area':
-            case 'input':
-                this._setValidBrowsingContextOnClick(el);
-                break;
         }
 
         // NOTE: we need to reprocess a tag client-side if it wasn't processed on the server.

diff --git a/test/client/data/page-navigation-watch/location-subject.html b/test/client/data/page-navigation-watch/location-subject.html
@@ -9,7 +9,6 @@
 <script type="text/javascript">
     var hammerhead = window['%hammerhead%'];
     hammerhead.start({ sessionId: 'unchangeableUrlSession' });
-    hammerhead.start();
 </script>
 </body>
 </html>
diff --git a/test/client/fixtures/sandbox/event/internal-listeners-test.js b/test/client/fixtures/sandbox/event/internal-listeners-test.js
@@ -399,67 +399,6 @@ test('only one of several handlers must be called (document handlers) (T233158)'
     document.removeEventListener(event, clickHandler, false);
 });
 
-test('only one of several handlers must be called (document handlers) (T233158)', function () {
-    var event               = 'click';
-    var clickHandlerCounter = 0;
-
-    var clickHandler = function () {
-        clickHandlerCounter++;
-    };
-
-    listeners.initElementListening(document, [event]);
-
-    listeners.addInternalEventListener(document, [event], function () {
-    });
-
-    var $document = $(document);
-
-    document.addEventListener(event, clickHandler, true);
-    document.addEventListener(event, clickHandler, true);
-    document.addEventListener(event, clickHandler, true);
-    dispatchEvent(document, event);
-    strictEqual(clickHandlerCounter, 1);
-
-    document.removeEventListener(event, clickHandler, true);
-    dispatchEvent(document, event);
-    strictEqual(clickHandlerCounter, 1);
-
-    $document.bind('click', clickHandler);
-    $document.bind('click', clickHandler);
-    dispatchEvent(document, event);
-    strictEqual(clickHandlerCounter, 3);
-
-    $document.unbind('click', clickHandler);
-    dispatchEvent(document, event);
-    strictEqual(clickHandlerCounter, 3);
-
-    $document.on('click', clickHandler);
-    $document.on('click', clickHandler);
-    dispatchEvent(document, event);
-    strictEqual(clickHandlerCounter, 5);
-
-    $document.off('click', clickHandler);
-    dispatchEvent(document, event);
-    strictEqual(clickHandlerCounter, 5);
-
-    document.addEventListener(event, clickHandler, true);
-    $document.bind('click', clickHandler);
-    $document.on('click', clickHandler);
-    dispatchEvent(document, event);
-    strictEqual(clickHandlerCounter, 8);
-    document.removeEventListener(event, clickHandler, true);
-    $document.unbind('click', clickHandler);
-    $document.off('click', clickHandler);
-
-    document.addEventListener(event, clickHandler, true);
-    document.addEventListener(event, clickHandler, false);
-    dispatchEvent(document, event);
-    strictEqual(clickHandlerCounter, 10);
-
-    document.removeEventListener(event, clickHandler, true);
-    document.removeEventListener(event, clickHandler, false);
-});
-
 test('only one of several handlers must be called (body handlers) (T233158)', function () {
     var event               = 'click';
     var clickHandlerCounter = 0;
-Original file line number
+Diff line change
@@ Expand Up / @@ -196,6 +196,7 @@ class Hammerhead { @@
             }
             this.sandbox.attach(this.win);
+            this.pageNavigationWatch.start();
         }
     }
@@ Expand Down @@