fine tuning #257, code review, jshint

js/background.js

@@ -115,7 +115,9 @@ return {
     noopFunc: function(){},
 
     apiErrorCount: 0,
-    elementPickerTarget: '',
+    contextMenuTarget: '',
+    contextMenuClientX: -1,
+    contextMenuClientY: -1,
 
     // so that I don't have to care for last comma
     dummy: 0

js/contentscript-end.js

@@ -766,3 +766,24 @@ var uBlockMessaging = (function(name){
 })();
 
 /******************************************************************************/
+/******************************************************************************/
+
+// To send mouse coordinates to context menu handler, as the chrome API fails
+// to provide the mouse position to context menu listeners.
+// This could be inserted in its own content script, but it's so simple that
+// I feel it's not worth the overhead.
+
+// Ref.: https://developer.mozilla.org/en-US/docs/Web/Events/contextmenu
+
+(function() {
+    var onContextMenu = function(ev) {
+        uBlockMessaging.tell({
+            what: 'contextMenuEvent',
+            clientX: ev.clientX,
+            clientY: ev.clientY
+        });
+    };
+    document.addEventListener('contextmenu', onContextMenu);
+})();
+
+/******************************************************************************/

js/contextmenu.js

@@ -19,7 +19,7 @@
     Home: https://github.com/gorhill/uBlock
 */
 
-/* global */
+/* global chrome, µBlock */
 
 /******************************************************************************/
 
@@ -63,10 +63,6 @@ var onContextMenuClicked = function(details, tab) {
         src = details.linkUrl;
     }
 
-    if ( src === '' ) {
-        return;
-    }
-
     µb.elementPickerExec(tab.id, tagName + '\t' + src);
 };
 
@@ -78,7 +74,7 @@ var toggleMenu = function(on) {
     var menuCreateDetails = {
         id: 'blockElement',
         title: chrome.i18n.getMessage('pickerContextMenuEntry'),
-        contexts: ['frame', 'link', 'image', 'video'],
+        contexts: ['page', 'frame', 'link', 'image', 'video'],
         documentUrlPatterns: ['https://*/*', 'http://*/*']
     };
 

js/element-picker.js

@@ -544,7 +544,6 @@ var candidateFromFilterChoice = function(filterChoice) {
 
     // Return path: the target element, then all siblings prepended
     var selector = [];
-    var filter;
     for ( ; slot < filters.length; slot++ ) {
         filter = filters[slot];
         selector.unshift(filter.replace(/^##/, ''));
@@ -669,27 +668,37 @@ var showDialog = function(options) {
 
 /******************************************************************************/
 
-var onSvgHovered = function(ev) {
-    if ( pickerPaused() ) {
-        return;
-    }
-
+var elementFromPoint = function(x, y) {
     svgRoot.style.pointerEvents = 'none';
-    var elem = document.elementFromPoint(ev.clientX, ev.clientY);
-    if ( elem == document.body || elem === document.documentElement ) {
+    var elem = document.elementFromPoint(x, y);
+    if ( elem === document.body || elem === document.documentElement ) {
         elem = null;
     }
-    highlightElements(elem ? [elem] : []);
     svgRoot.style.pointerEvents = 'auto';
+    return elem;
 };
 
 /******************************************************************************/
 
-var onSvgClicked = function() {
+var onSvgHovered = function(ev) {
     if ( pickerPaused() ) {
         return;
     }
-    filtersFromElement(targetElements[0]);
+    var elem = elementFromPoint(ev.clientX, ev.clientY);
+    highlightElements(elem ? [elem] : []);
+};
+
+/******************************************************************************/
+
+var onSvgClicked = function(ev) {
+    if ( pickerPaused() ) {
+        return;
+    }
+    var elem = elementFromPoint(ev.clientX, ev.clientY);
+    if ( elem === null ) {
+        return;
+    }
+    filtersFromElement(elem);
     showDialog();
 };
 
@@ -709,7 +718,7 @@ var onKeyPressed = function(ev) {
 // May need to dynamically adjust the height of the overlay + new position
 // of highlighted elements.
 
-var onScrolled = function(ev) {
+var onScrolled = function() {
     var newHeight = this.scrollY + this.innerHeight;
     if ( newHeight > svgHeight ) {
         svgHeight = newHeight;
@@ -971,8 +980,19 @@ messaging.ask({ what: 'elementPickerArguments' }, function(details) {
     }
 
     // Auto-select a specific target, if any, and if possible
-    var targetElement = details.targetElement || '';
-    var pos = targetElement.indexOf('\t');
+    var elem;
+
+    // Try using mouse position
+    if ( details.clientX !== -1 ) {
+        elem = elementFromPoint(details.clientX, details.clientY);
+        filtersFromElement(elem);
+        showDialog();
+        return;
+    }
+
+    // No mouse position available, use suggested target
+    var target = details.target || '';
+    var pos = target.indexOf('\t');
     if ( pos === -1 ) {
         return;
     }
@@ -983,28 +1003,27 @@ messaging.ask({ what: 'elementPickerArguments' }, function(details) {
         'video': 'src',
         'audio': 'src'
     };
-    var tagName = targetElement.slice(0, pos);
-    var url = targetElement.slice(pos + 1);
+    var tagName = target.slice(0, pos);
+    var url = target.slice(pos + 1);
     var attr = srcAttrMap[tagName];
     if ( attr === undefined ) {
         return;
     }
     var elems = document.querySelectorAll(tagName + '[' + attr + ']');
     var i = elems.length;
-    var elem, src;
+    var src;
     while ( i-- ) {
         elem = elems[i];
-        src = elem.getAttribute(attr);
-        if ( src === null || src === '' ) {
+        src = elem[attr];
+        if ( typeof src !== 'string' || src === '' ) {
+            continue;
+        }
+        if ( src !== url ) {
             continue;
         }
-        urlNormalizer.href = src;
-        src = urlNormalizer.href;
-        if ( src === url ) {
         filtersFromElement(elem);
         showDialog({ modifier: true });
-            break;
-        }
+        return;
     }
 });
 

js/messaging-handlers.js

@@ -307,9 +307,13 @@ var onMessage = function(request, sender, callback) {
                     cosmeticFilters: chrome.i18n.getMessage('pickerCosmeticFilters'),
                     cosmeticFiltersHint: chrome.i18n.getMessage('pickerCosmeticFiltersHint')
                 },
-                targetElement: µb.elementPickerTarget
+                target: µb.contextMenuTarget,
+                clientX: µb.contextMenuClientX,
+                clientY: µb.contextMenuClientY
             };
-            µb.elementPickerTarget = '';
+            µb.contextMenuTarget = '';
+            µb.contextMenuClientX = -1;
+            µb.contextMenuClientY = -1;
             break;
 
         case 'createUserFilter':

js/messaging.js

@@ -137,13 +137,15 @@ var onMessage = function(request, port) {
 // Default is for commonly used messages.
 
 function defaultHandler(request, sender, callback) {
+    var µb = µBlock;
+
     // Async
     switch ( request.what ) {
         case 'getAssetContent':
-            return µBlock.assets.getLocal(request.url, callback);
+            return µb.assets.getLocal(request.url, callback);
 
         case 'loadUbiquitousAllowRules':
-            return µBlock.loadUbiquitousWhitelists();
+            return µb.loadUbiquitousWhitelists();
 
         default:
             break;
@@ -153,28 +155,33 @@ function defaultHandler(request, sender, callback) {
     var response;
 
     switch ( request.what ) {
+        case 'contextMenuEvent':
+            µb.contextMenuClientX = request.clientX;
+            µb.contextMenuClientY = request.clientY;
+            break;
+
         case 'forceReloadTab':
-            µBlock.forceReload(request.pageURL);
+            µb.forceReload(request.pageURL);
             break;
 
         case 'getUserSettings':
-            response = µBlock.userSettings;
+            response = µb.userSettings;
             break;
 
         case 'gotoExtensionURL':
-            µBlock.utils.gotoExtensionURL(request.url);
+            µb.utils.gotoExtensionURL(request.url);
             break;
 
         case 'gotoURL':
-            µBlock.utils.gotoURL(request);
+            µb.utils.gotoURL(request);
             break;
 
         case 'reloadAllFilters':
-            µBlock.reloadPresetBlacklists(request.switches, request.update);
+            µb.reloadPresetBlacklists(request.switches, request.update);
             break;
 
         case 'userSettings':
-            response = µBlock.changeUserSettings(request.name, request.value);
+            response = µb.changeUserSettings(request.name, request.value);
             break;
 
         default:

js/ublock.js

@@ -246,7 +246,7 @@
 
 µBlock.elementPickerExec = function(tabId, targetElement) {
     this.elementPickerTarget = targetElement || '';
-    chrome.tabs.executeScript(tabId, { file: 'js/element-picker.js' });
+    this.XAL.injectScript(tabId, { file: 'js/element-picker.js' });
 };
 
 /******************************************************************************/

js/xal.js

@@ -53,6 +53,12 @@ exports.setIcon = function(id, imgDict, overlayStr) {
 
 /******************************************************************************/
 
+exports.injectScript = function(id, details) {
+    chrome.tabs.executeScript(id, details);
+};
+
+/******************************************************************************/
+
 return exports;
 
 /******************************************************************************/