Skip to content

Loading…

Add a "Block Element" context menu command to the Firefox Network tab #1324

Merged
merged 2 commits into from

3 participants

@AlexVallat
Collaborator

What do you think, would this make a good addition? In my view it complements the Block Element command added to the Inspector tab with #1211 well, as when I want to create a new static filter, I usually end up copying and pasting URLs from here anyway.

block element from network tab

@AlexVallat AlexVallat Add a "Block Element" context menu command to the Firefox Network tab…
… (to match the one in the Inspector tab)
c98ff26
@chrisaljoudi

@AlexVallat Nice, but this doesn't block the element per se, but rather the URL of the request, correct?

If so, I think it'd be more appropriate to call it "Block Resource".

@AlexVallat
Collaborator

Hmm, yes, fair point. I was just reusing the same menu command in a different spot, but it is the URL that it applies to. That would mean a new translatable string, though...

@chrisaljoudi

@AlexVallat right.

The thing is, we'd hate for it be to be misleading.

Here's another idea: it could just say "Block [filename]". So for example, "Block transparent-1211.png".

That avoids the translation issue and actually is somewhat clearer (I think).

@AlexVallat
Collaborator

Yes, that's a possibility, but there's not often such a clean "filename" to use at the end of a URL. We could do some guesswork, use the last part of the path, strip off query string and anchor parts, and so on. Put in a length limit to avoid extra-wide menu text.

In the end, we'd still have the translation issue because we'd need the string "Block %s" translated!

@chrisaljoudi

@AlexVallat ah, true.

Well, it's fine to add a translatable string for "Block Resource".

Though, I just thought of this: wasn't there an issue somewhere that talked about something very similar to this in uBlock's own network request logger? (I'll see if I can find it).

@gorhill

@chrisaljoudi

wasn't there an issue somewhere

Here: #68.

@AlexVallat
Collaborator

I think it would be a generally useful thing to be able to create a filter from the uBlock network request logger, yes. Trickier to do, though, as it's on a different page (so would need either a different UI for adding, or to switch to the tab to show the element picker). Also, the logger contains more than just network requests (inline-script, for example), so you'd have to either support those too, or provide feedback as to why you can't block them. I'm not saying it shouldn't be done, just that it's a different job to this one!

I'll make the change to introduce a new string when I get a chance. Any reason you prefer "Block Resource" to "Block URL", which would have been my first guess?

@AlexVallat AlexVallat Change net monitor menu label to be independent, and to be "Block Res…
…ource"
424f131
@chrisaljoudi

@AlexVallat Sorry for not replying earlier.

Block URL also works, but I thought "Block Resource" works better with the terminology of the network request logger and would translate well to uBlock's own request logger.

@AlexVallat
Collaborator

@chrisaljoudi No problem. Are you happy for this to be merged as it is now, then?

@AlexVallat
Collaborator

@chrisaljoudi: Just want to check, do you specifically not want this to be in 0.9.4.0? I know it's a bit of a change to the element picker code, but I'm pretty sure it doesn't break existing functionality.

@chrisaljoudi
Owner

@AlexVallat I'm really sorry for having gone inactive over the weekend and such (unexpected medical issues).

My thought was to finish implementing the same feature in the request logger (so one could add rules right from there) and have them both in the same release.

Sorry for not including it in 0.9.4.0. I hope that's alright.

@AlexVallat
Collaborator

@chrisaljoudi Oh, I'm sorry to hear that, I hope you're feeling better.

No, that's fine, as long as it's not been forgotten!

@chrisaljoudi chrisaljoudi merged commit 992af97 into master

2 checks passed

Details continuous-integration/travis-ci/pr The Travis CI build passed
Details continuous-integration/travis-ci/push The Travis CI build passed
@AlexVallat AlexVallat deleted the NetMonitor_Block_Element branch
@AlexVallat AlexVallat added a commit to AlexVallat/uBlock that referenced this pull request
@AlexVallat AlexVallat Cherry pick Network inspector element picker #1324 7141239
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 26, 2015
  1. @AlexVallat

    Add a "Block Element" context menu command to the Firefox Network tab…

    AlexVallat committed
    … (to match the one in the Inspector tab)
Commits on Apr 29, 2015
  1. @AlexVallat

    Change net monitor menu label to be independent, and to be "Block Res…

    AlexVallat committed
    …ource"
This page is out of date. Refresh to see the latest.
View
26 platform/firefox/vapi-background.js
@@ -1944,10 +1944,10 @@ vAPI.contextMenu.displayMenuItem = function({target}) {
/******************************************************************************/
-vAPI.contextMenu.createContextMenuItem = function(doc) {
+vAPI.contextMenu.createContextMenuItem = function(doc, labelOverride) {
var menuitem = doc.createElement('menuitem');
menuitem.setAttribute('id', this.menuItemId);
- menuitem.setAttribute('label', this.menuLabel);
+ menuitem.setAttribute('label', labelOverride || this.menuLabel);
menuitem.setAttribute('image', vAPI.getURL('img/browsericons/icon16.svg'));
menuitem.setAttribute('class', 'menuitem-iconic');
return menuitem;
@@ -2019,7 +2019,7 @@ vAPI.contextMenu.registerForWebInspector = function(eventName, toolbox, panel) {
selectedNodeFront = selectedNodeFront.parentNode();
}
if (selectedNodeFront) {
- selectedNodeFront.getUniqueSelector().then(selector => µBlock.elementPickerExec(vAPI.tabs.getTabId(panel.browser), selector));
+ selectedNodeFront.getUniqueSelector().then(selector => µBlock.elementPickerExec(vAPI.tabs.getTabId(panel.browser), { type: 'element', value: selector}));
// Turn off 3D view, if it's turned on.
if (tiltButton && tiltButton.checked) {
@@ -2032,6 +2032,24 @@ vAPI.contextMenu.registerForWebInspector = function(eventName, toolbox, panel) {
}
}
+vAPI.contextMenu.registerForNetMonitor = function(eventName, toolbox, panel) {
+ var doc = panel.panelWin.document;
+ var menuPopup = doc.getElementById("network-request-popup");
+ var insertBeforeMenuItem = doc.getElementById("request-menu-context-separator");
+
+ if (menuPopup && insertBeforeMenuItem) {
+ var menuitem = vAPI.contextMenu.createContextMenuItem(doc, vAPI.i18n('netMonitorContextMenuEntry'));
+ menuitem.addEventListener('command', function() {
+ var selectedRequest = panel.panelWin.NetMonitorView.RequestsMenu.selectedAttachment;
+ if (selectedRequest) {
+ µBlock.elementPickerExec(vAPI.tabs.getTabId(toolbox.target.tab), { type: 'url', value: selectedRequest.url });
+ }
+ });
+
+ menuPopup.insertBefore(menuitem, insertBeforeMenuItem);
+ }
+}
+
/******************************************************************************/
vAPI.contextMenu.create = function(details, callback) {
@@ -2086,6 +2104,7 @@ vAPI.contextMenu.create = function(details, callback) {
if (this.gDevTools) {
this.gDevTools.on("inspector-ready", this.registerForWebInspector);
+ this.gDevTools.on("netmonitor-ready", this.registerForNetMonitor);
}
}
@@ -2103,6 +2122,7 @@ vAPI.contextMenu.remove = function() {
if (!vAPI.fennec && this.gDevTools) {
this.gDevTools.off("inspector-ready", this.registerForWebInspector);
+ this.gDevTools.off("netmonitor-ready", this.registerForNetMonitor);
}
this.menuItemId = null;
View
4 src/_locales/en/messages.json
@@ -147,6 +147,10 @@
"message":"Block element",
"description":"English: Block element"
},
+ "netMonitorContextMenuEntry":{
+ "message":"Block resource",
+ "description":"English: Block resource"
+ },
"settingsCollapseBlockedPrompt":{
"message":"Hide placeholders of blocked elements",
"description":"English: Hide placeholders of blocked elements"
View
2 src/js/background.js
@@ -138,7 +138,7 @@ return {
contextMenuClientX: -1,
contextMenuClientY: -1,
- epickerTargetElementSelector: null,
+ epickerTarget: null,
epickerEprom: null,
// so that I don't have to care for last comma
View
48 src/js/element-picker.js
@@ -326,13 +326,18 @@ var netFilterFromElement = function(elem, out) {
if ( src.length === 0 ) {
return;
}
+
+ netFilterFromUrl(src, out);
+}
+
+var netFilterFromUrl = function(url, out) {
// Remove fragment
- var pos = src.indexOf('#');
+ var pos = url.indexOf('#');
if ( pos !== -1 ) {
- src = src.slice(0, pos);
+ url = url.slice(0, pos);
}
- var filter = src.replace(/^https?:\/\//, '||');
+ var filter = url.replace(/^https?:\/\//, '||');
// Anchor absolute filter to hostname
out.push(filter);
@@ -344,7 +349,7 @@ var netFilterFromElement = function(elem, out) {
}
// Suggest a filter which is a result of combining more than one URL.
- netFilterFromUnion(src, out);
+ netFilterFromUnion(url, out);
};
var netFilterSources = {
@@ -472,6 +477,14 @@ var filtersFromElement = function(elem) {
/******************************************************************************/
+var filtersFromUrl = function(url) {
+ netFilterCandidates.length = 0;
+ cosmeticFilterCandidates.length = 0;
+ netFilterFromUrl(url, netFilterCandidates);
+};
+
+/******************************************************************************/
+
var elementsFromFilter = function(filter) {
var out = [];
@@ -874,20 +887,23 @@ var startPicker = function(details) {
highlightElements([], true);
- var elem = null;
-
- // If a target element was provided, use it
- if (details.targetElementSelector) {
- elem = document.querySelector(details.targetElementSelector);
- }
-
- // Try using mouse position
- if (!elem && details.clientX !== -1) {
- elem = elementFromPoint(details.clientX, details.clientY);
+ // If a target was provided, use it
+ if (details.target) {
+ if (details.target.type === 'element') {
+ filtersFromElement(document.querySelector(details.target.value));
+ } else if (details.target.type === 'url') {
+ filtersFromUrl(details.target.value);
+ } else {
+ console.error('uBlock> unknown element picker target details type: %s', details.target.type);
+ }
+ } else {
+ // Try using mouse position
+ if (details.clientX !== -1) {
+ filtersFromElement(elementFromPoint(details.clientX, details.clientY));
+ }
}
- if (elem !== null) {
- filtersFromElement(elem);
+ if (netFilterCandidates.length > 0 || cosmeticFilterCandidates.length > 0) {
showDialog();
}
};
View
2 src/js/messaging.js
@@ -558,7 +558,7 @@ var onMessage = function(request, sender, callback) {
callback({
frameContent: this.responseText.replace(reStrings, replacer),
- targetElementSelector: µb.epickerTargetElementSelector,
+ target: µb.epickerTarget,
clientX: µb.contextMenuClientX,
clientY: µb.contextMenuClientY,
eprom: µb.epickerEprom
View
4 src/js/ublock.js
@@ -276,8 +276,8 @@ var matchWhitelistDirective = function(url, hostname, directive) {
/******************************************************************************/
Block.elementPickerExec = function(tabId, targetElementSelector) {
- this.epickerTargetElementSelector = targetElementSelector;
Block.elementPickerExec = function(tabId, target) {
+ this.epickerTarget = target;
vAPI.tabs.injectScript(tabId, { file: 'js/element-picker.js' });
};
View
2 tools/make-firefox-meta.py
@@ -24,7 +24,7 @@ def mkdirs(path):
target_locale_dir = pj(build_dir, 'locale')
language_codes = []
descriptions = OrderedDict({})
-title_case_strings = ['pickerContextMenuEntry']
+title_case_strings = ['pickerContextMenuEntry', 'netMonitorContextMenuEntry']
for alpha2 in sorted(os.listdir(source_locale_dir)):
locale_path = pj(source_locale_dir, alpha2, 'messages.json')
Something went wrong with that request. Please try again.